2015-06-03 21:01:39

by Ilya Faenson

[permalink] [raw]
Subject: [PATCH 0/5] Broadcom Bluetooth UART device driver

This is a merge of the Broadcom Bluetooth UART logic with
the latest line discipline and protocol enhancements implemented
by Frederic Danis of Intel.

Ilya Faenson (5):
Broadcom Bluetooth UART Device Tree bindings
Intel based H4 line discipline enhancements
Broadcom Bluetooth UART Platform Driver
Broadcom Bluetooth protocol UART support
BlueZ Broadcom UART Protocol

.../devicetree/bindings/net/bluetooth/btbcm.txt | 82 +++
drivers/bluetooth/Kconfig | 9 +
drivers/bluetooth/Makefile | 1 +
drivers/bluetooth/btbcm.c | 142 ++++-
drivers/bluetooth/btbcm.h | 21 +-
drivers/bluetooth/btbcm_uart.c | 673 +++++++++++++++++++++
drivers/bluetooth/btbcm_uart.h | 89 +++
drivers/bluetooth/hci_bcm.c | 481 ++++++++++++++-
drivers/bluetooth/hci_ldisc.c | 110 +++-
drivers/bluetooth/hci_uart.h | 6 +
10 files changed, 1565 insertions(+), 49 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
create mode 100755 drivers/bluetooth/btbcm_uart.c
create mode 100755 drivers/bluetooth/btbcm_uart.h

--
1.9.1


2015-06-07 07:39:18

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 1/5] Broadcom Bluetooth UART Device Tree bindings

Hi Arend,

>>>>> Device Tree bindings to configure the Broadcom Bluetooth UART device.
>>>>> Updated not to use CamelCase and to use Intel style "oper-speed".
>>>>>
>>>>> Signed-off-by: Ilya Faenson<[email protected]>
>>>>> ---
>>>>> .../devicetree/bindings/net/bluetooth/btbcm.txt | 82 ++++++++++++++++++++++
>>>>> 1 file changed, 82 insertions(+)
>>>>> create mode 100644 Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
>>>>> new file mode 100644
>>>>> index 0000000..2679819
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
>>>>> @@ -0,0 +1,82 @@
>>>>> +btbcm
>>>>> +------
>>>>> +
>>>>> +Required properties:
>>>>> +
>>>>> + - compatible : must be "brcm,brcm-bt-uart".
>>>>> + - tty : tty device connected to this Bluetooth device.
>>>>> +
>>>>> +Optional properties:
>>>>> +
>>>>> + - bt-host-wake-gpios : bt-host-wake input GPIO to be used as an interrupt.
>>>>> +
>>>>> + - bt-wake-gpios : bt-wake output GPIO to be used to suspend / resume device.
>>>>> +
>>>>> + - bt-reg-on-gpios : reg-on output GPIO to be used to power device on/off.
>>>>> +
>>>>> + - oper-speed : Bluetooth device operational baud rate.
>>>>> + Default: 3000000.
>>>>> +
>>>>> + - manual-fc : flow control UART in suspend / resume scenarios.
>>>>> + Default: 0.
>>>>> +
>>>>> + - configure-sleep : configure suspend / resume flag.
>>>>> + Default: false.
>>>>> +
>>>>> + - configure-audio : configure platform PCM SCO flag.
>>>>> + Default: false.
>>>>> +
>>>>> + - pcm-clockmode : PCM clock mode. 0-slave, 1-master.
>>>>> + Default: 0.
>>>>> +
>>>>> + - pcm-fillmethod : PCM fill method. 0 to 3.
>>>>> + Default: 2.
>>>>> +
>>>>> + - pcm-fillnum : PCM number of fill bits. 0 to 3.
>>>>> + Default: 0.
>>>>> +
>>>>> + - pcm-fillvalue : PCM fill value. 0 to 7.
>>>>> + Default: 3.
>>>>> +
>>>>> + - pcm-incallbitclock : PCM interface rate. 0-128Kbps, 1-256Kbps, 2-512Kbps,
>>>>> + 3-1024Kbps, 4-2048Kbps.
>>>>> + Default: 0.
>>>>> +
>>>>> + - pcm-lsbfirst : PCM LSB first. 0 or 1.
>>>>> + Default: 0.
>>>>> +
>>>>> + - pcm-rightjustify : PCM Justify. 0-left, 1-right.
>>>>> + Default: 0.
>>>>> +
>>>>> + - pcm-routing : PCM routing. 0-PCM, 1-SCO over HCI.
>>>>> + Default: 0.
>>>>> +
>>>>> + - pcm-shortframesync : PCM sync. 0-short, 1-long.
>>>>> + Default: 0.
>>>>> +
>>>>> + - pcmsyncmode : PCM sync mode. 0-slave, 1-master.
>>>>> + Default: 0.
>>>>> +
>>>>> +
>>>>> +Example:
>>>>> +
>>>>> + brcm4354_bt_uart: brcm4354-bt-uart {
>>>>
>>>> at some point you have to explain to me if Broadcom prefers BCM or BRCM. I can not make heads or tail out of it. My current explanation is the it is BCM for the Bluetooth guys and BRCM for the WiFi guys ;)
>>>
>>> That's a nice explanation ;-) Maybe I can give it a shot. First, two facts: 1) BRCM is our stock symbol, and 2) our device names are prefixed by BCM. So in devicetree specifications we specified the vendor prefix as BRCM and (try to) stick to that. The above clearly refers to a device so it would probably be better to use bcm4354... there.
>>>
>>>>> + compatible = "brcm,brcm-bt-uart";
>>>
>>> The compatible string indeed starts with vendor prefix "brcm,".
>>>
>>>>> + bt-wake-gpios =<&gpio4 30 GPIO_ACTIVE_HIGH>;
>>>>> + bt-host-wake-gpios =<&gpio4 31 GPIO_ACTIVE_HIGH>;
>>>>> + tty = "ttyS0”;
>>>>
>>>> This tty one is the one we need to figure out. Could we reference by some node information instead of the assigned device name. If we can reference something in struct device that we know is more consistent, that would be perfect. I mean if the kernel for reason decides to enumerate things in a different order then this might be ttyS1 out of a sudden.
>>>
>>> I see your point, but not sure what to do here either. In devicetree you could reference the uart node, but I don't know if there is an api in the kernel to obtain tty associated with a uart port. I could not find it doing quick glance in include/linux/tty.h although the linkage is obviously there in the data types.
>>
>> I wonder if we get this done via ACPI in the first place. However that is something for Fred to figure out.
>>
>> For device tree, I bet we need to extend either the TTY subsystem or really start pushing towards the tty-slave idea that has been floating around.
>
> The tty-slave idea popped up already in review comments. We are wondering if it is still floating or whether things are starting to settle in that area. Specially for the devicetree spec, which is supposed to be a stable ABI, we probably need to take that into account if that is indeed the way things are moving.
>
> Ilya, this is the most recent thing I found:
>
> http://thread.gmane.org/gmane.linux.kernel/1949724
>
> The uart-slave idea seems to sit between tty and uart dealing with powering the device. Not sure how that fits our need. It mentions something about being able to intervene in any tty-uart interaction so there could bt stuff that could be done in those hooks.

it would be actually nice if it also considers ACPI from the start. There seems to be enough description in the ACPI tables that it could just work.

The way I see it is that having a vendor specific driver that facilitates the power handling seems like a nice and clean design. However right now it is limited to TTY open/close operations. Do we need extra runtime power handling that the UART slave should also be able to facilitate.

The GPS case seems to be easy. You open and close the TTY and then get NMEA sentences or not. That is an easy mapping from userspace and makes total sense. For Bluetooth however the open normally happens when you enable Bluetooth. Meaning that it can only facility the master switch of on and off. The runtime aspects should be considered as well.

That said, the master switch for on/off would be a huge step into dealing with chips hidden behind UARTs.

Regards

Marcel


2015-06-06 18:25:41

by Ilya Faenson

[permalink] [raw]
Subject: RE: [PATCH 2/5] Intel based H4 line discipline enhancements

Hi Marcel,

-----Original Message-----
From: Marcel Holtmann [mailto:[email protected]]=20
Sent: Saturday, June 06, 2015 11:51 AM
To: Ilya Faenson
Cc: [email protected]; Arend Van Spriel
Subject: Re: [PATCH 2/5] Intel based H4 line discipline enhancements

Hi Ilya,

the mailing list has a strict policy of no top-posting. So please adhere to=
that. I know it is hard with corporate email systems, but for mailing list=
emails I am policing this.

IF: Will do, thanks.

> Appreciate you applying Fred's patches. I will now put together and post =
similar patches against the bluetooth-next proper.
>=20
> Sorry for many minor changes. I just happen to run the checkpatch script =
automatically prior to publishing the patches (Arend taught me good Linux p=
ractices :-)) and that results in those spaces getting deleted et al.

Please keep in that you need --strict actually. We have to adhere to the ne=
twork subsystem rules. Also some of the driver code is pretty old. You will=
find violations. Feel free to fix them, but please do it in a separate pat=
ch before your changes.

IF: Understood, thanks.

Regards

Marcel

2015-06-06 18:24:11

by Ilya Faenson

[permalink] [raw]
Subject: RE: [PATCH 2/5] Intel based H4 line discipline enhancements

SGkgQXJlbmQsDQoNCi0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQpGcm9tOiBBcmVuZCB2YW4g
U3ByaWVsIFttYWlsdG86YXJlbmRAYnJvYWRjb20uY29tXSANClNlbnQ6IFNhdHVyZGF5LCBKdW5l
IDA2LCAyMDE1IDExOjQxIEFNDQpUbzogSWx5YSBGYWVuc29uDQpDYzogTWFyY2VsIEhvbHRtYW5u
OyBsaW51eC1ibHVldG9vdGhAdmdlci5rZXJuZWwub3JnDQpTdWJqZWN0OiBSZTogW1BBVENIIDIv
NV0gSW50ZWwgYmFzZWQgSDQgbGluZSBkaXNjaXBsaW5lIGVuaGFuY2VtZW50cw0KDQpPbiAwNi8w
Ni8xNSAxNzozMywgSWx5YSBGYWVuc29uIHdyb3RlOg0KPiBIaSBNYXJjZWwsDQo+DQo+IEFwcHJl
Y2lhdGUgeW91IGFwcGx5aW5nIEZyZWQncyBwYXRjaGVzLiBJIHdpbGwgbm93IHB1dCB0b2dldGhl
ciBhbmQgcG9zdCBzaW1pbGFyIHBhdGNoZXMgYWdhaW5zdCB0aGUgYmx1ZXRvb3RoLW5leHQgcHJv
cGVyLg0KPg0KPiBTb3JyeSBmb3IgbWFueSBtaW5vciBjaGFuZ2VzLiBJIGp1c3QgaGFwcGVuIHRv
IHJ1biB0aGUgY2hlY2twYXRjaCBzY3JpcHQgYXV0b21hdGljYWxseSBwcmlvciB0byBwdWJsaXNo
aW5nIHRoZSBwYXRjaGVzIChBcmVuZCB0YXVnaHQgbWUgZ29vZCBMaW51eCBwcmFjdGljZXMgOi0p
KSBhbmQgdGhhdCByZXN1bHRzIGluIHRob3NlIHNwYWNlcyBnZXR0aW5nIGRlbGV0ZWQgZXQgYWwu
DQoNCkFjdHVhbGx5LCBjaGVja3BhdGNoIGJ5IGRlZmF1bHQgb25seSBkb2VzIHRoZSBjaGVjayBh
bmQganVzdCB0ZWxsIHlvdSANCndoYXQgaXMgd3JvbmcuIE5vcm1hbGx5LCBpdCB3b3VsZCBvbmx5
IHdhcm4gYWJvdXQgdGhlIHRoaW5nIHlvdXIgcGF0Y2ggDQptb2RpZmllZC4gVW5sZXNzIHlvdSBy
YW4gY2hlY2twYXRjaCBvbiBzb3VyY2UgZmlsZS4NCg0KSUY6IFllcywgSSd2ZSBiZWVuIHJ1bm5p
bmcgY2hlY2twYXRjaCBvbiBzb3VyY2UgZmlsZXMgdG9vLg0KDQpSZWdhcmRzLA0KQXJlbmQNCg0K
PiBUaGFua3MsDQo+ICAgLUlseWENCj4NCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4g
RnJvbTogTWFyY2VsIEhvbHRtYW5uIFttYWlsdG86bWFyY2VsQGhvbHRtYW5uLm9yZ10NCj4gU2Vu
dDogU2F0dXJkYXksIEp1bmUgMDYsIDIwMTUgMjozNyBBTQ0KPiBUbzogSWx5YSBGYWVuc29uDQo+
IENjOiBsaW51eC1ibHVldG9vdGhAdmdlci5rZXJuZWwub3JnDQo+IFN1YmplY3Q6IFJlOiBbUEFU
Q0ggMi81XSBJbnRlbCBiYXNlZCBINCBsaW5lIGRpc2NpcGxpbmUgZW5oYW5jZW1lbnRzDQo+DQo+
IEhpIElseWEsDQo+DQo+PiBUaGlzIGlzIGxhcmdlbHkgZW5oYW5jZW1lbnRzIGltcGxlbWVudGVk
IGJ5IEZyZWRlcmljIERhbmlzIG9mIEludGVsLg0KPj4gSSd2ZSBhbHNvIGFkZGVkIHRoZSBhYmls
aXR5IHRvIGZsb3cgY29udHJvbCB0aGUgVUFSVCBhbmQgaW1wcm92ZWQgdGhlDQo+PiBVQVJUIGJh
dWQgcmF0ZSBzZXR0aW5nIHNvbWUuDQo+DQo+IEkgYXBwbGllZCBwYXJ0cyBvZiBGcmVk4oCZcyBw
YXRjaGVzIGFuZCBzbyBwbGVhc2UgcmViYXNlIG9uIHRvcCBvZiB0aGVtLiBNYWtlcyBpdCBhIGxv
dCBlYXNpZXIgZm9yIG1lIHRvIHJldmlldyBjaGFuZ2VzIGluc3RlYWQgb2YgZmlndXJpbmcgb3V0
IHdoYXQgY29tZXMgZnJvbSB5b3UgYW5kIHdoYXQgY29tZXMgZnJvbSBGcmVkLg0KPg0KPj4NCj4+
IFNpZ25lZC1vZmYtYnk6IElseWEgRmFlbnNvbjxpZmFlbnNvbkBicm9hZGNvbS5jb20+DQo+PiAt
LS0NCj4+IGRyaXZlcnMvYmx1ZXRvb3RoL2hjaV9sZGlzYy5jIHwgMTEwICsrKysrKysrKysrKysr
KysrKysrKysrKysrKysrKysrKysrKysrKystLQ0KPj4gZHJpdmVycy9ibHVldG9vdGgvaGNpX3Vh
cnQuaCAgfCAgIDYgKysrDQo+PiAyIGZpbGVzIGNoYW5nZWQsIDExMyBpbnNlcnRpb25zKCspLCAz
IGRlbGV0aW9ucygtKQ0KPj4NCj4+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2JsdWV0b290aC9oY2lf
bGRpc2MuYyBiL2RyaXZlcnMvYmx1ZXRvb3RoL2hjaV9sZGlzYy5jDQo+PiBpbmRleCA1YzlhNzNm
Li41OGRjYjI0IDEwMDY0NA0KPj4gLS0tIGEvZHJpdmVycy9ibHVldG9vdGgvaGNpX2xkaXNjLmMN
Cj4+ICsrKyBiL2RyaXZlcnMvYmx1ZXRvb3RoL2hjaV9sZGlzYy5jDQo+PiBAQCAtMSw0ICsxLDQg
QEANCj4+IC0vKg0KPj4gKy8qXw0KPg0KPiBGdW5reSBjaGFuZ2UgOykNCj4NCj4+ICAgKg0KPj4g
ICAqICBCbHVldG9vdGggSENJIFVBUlQgZHJpdmVyDQo+PiAgICoNCj4+IEBAIC0yNTYsNyArMjU2
LDggQEAgc3RhdGljIGludCBoY2lfdWFydF9zZW5kX2ZyYW1lKHN0cnVjdCBoY2lfZGV2ICpoZGV2
LCBzdHJ1Y3Qgc2tfYnVmZiAqc2tiKQ0KPj4gCWlmICghdGVzdF9iaXQoSENJX1JVTk5JTkcsJmhk
ZXYtPmZsYWdzKSkNCj4+IAkJcmV0dXJuIC1FQlVTWTsNCj4+DQo+PiAtCUJUX0RCRygiJXM6IHR5
cGUgJWQgbGVuICVkIiwgaGRldi0+bmFtZSwgYnRfY2Ioc2tiKS0+cGt0X3R5cGUsIHNrYi0+bGVu
KTsNCj4+ICsJQlRfREJHKCIlczogdHlwZSAlZCBsZW4gJWQiLCBoZGV2LT5uYW1lLCBidF9jYihz
a2IpLT5wa3RfdHlwZSwNCj4+ICsJICAgICAgIHNrYi0+bGVuKTsNCj4NCj4gSW4gY2FzZSB5b3Ug
c3BvdCBjb2Rpbmcgc3R5bGUgZXJyb3JzLCBwbGVhc2UganVzdCBzZW5kIHRoZW0gYXMgdGlueSBj
bGVhbnVwIHBhdGNoZXMuIEkgY2FuIGVhc2lseSBhcHBseSB0aGVtIG91dCBvZiBvcmRlciBhbmQg
d2UgZ2V0IG9mIHRoaXMuIE1ha2VzIHJldmlldyBhIGxvdCBlYXNpZXIuDQo+DQo+Pg0KPj4gCWh1
LT5wcm90by0+ZW5xdWV1ZShodSwgc2tiKTsNCj4+DQo+PiBAQCAtMjY1LDExICsyNjYsMTE0IEBA
IHN0YXRpYyBpbnQgaGNpX3VhcnRfc2VuZF9mcmFtZShzdHJ1Y3QgaGNpX2RldiAqaGRldiwgc3Ry
dWN0IHNrX2J1ZmYgKnNrYikNCj4+IAlyZXR1cm4gMDsNCj4+IH0NCj4+DQo+PiArdm9pZCBoY2lf
dWFydF9mbG93X2NvbnRyb2xfZGV2aWNlKHN0cnVjdCBoY2lfdWFydCAqaHUpDQo+PiArew0KPj4g
KwlzdHJ1Y3QgdHR5X3N0cnVjdCAqdHR5ID0gaHUtPnR0eTsNCj4+ICsJc3RydWN0IGt0ZXJtaW9z
IGt0ZXJtaW9zOw0KPj4gKwlpbnQgc3RhdHVzOw0KPj4gKwl1bnNpZ25lZCBpbnQgc2V0ID0gMDsN
Cj4+ICsJdW5zaWduZWQgaW50IGNsZWFyID0gMDsNCj4+ICsNCj4+ICsJLyogRGlzYWJsZSBoYXJk
d2FyZSBmbG93IGNvbnRyb2wgKi8NCj4+ICsJa3Rlcm1pb3MgPSB0dHktPnRlcm1pb3M7DQo+PiAr
CWt0ZXJtaW9zLmNfY2ZsYWcmPSB+Q1JUU0NUUzsNCj4+ICsJc3RhdHVzID0gdHR5X3NldF90ZXJt
aW9zKHR0eSwma3Rlcm1pb3MpOw0KPj4gKwlpZiAoc3RhdHVzKQ0KPj4gKwkJQlRfREJHKCIlcyBk
aXMgZmMgZmFpbHVyZSAlZCIsIF9fZnVuY19fLCBzdGF0dXMpOw0KPj4gKwllbHNlDQo+PiArCQlC
VF9EQkcoIiVzIGh3IGZjIGRpc2FibGVkIiwgX19mdW5jX18pOw0KPg0KPiBMZXRzIGRvIGl0IGxp
a2UgdGhpczoNCj4NCj4gCUJUX0RCRyjigJxEaXNhYmxpbmcgaGFyZHdhcmUgZmxvdyBjb250cm9s
OiAlc+KAnSwgc3RhdHVzID8g4oCcZmFpbGVk4oCdIDog4oCcc3VjY2Vzc+KAnSk7DQo+DQo+PiAr
DQo+PiArCS8qIENsZWFyIFJUUyB0byBwcmV2ZW50IHRoZSBkZXZpY2UgZnJvbSBzZW5kaW5nICov
DQo+PiArCS8qIChtb3N0IFBDcyBuZWVkIE9VVDIgdG8gZW5hYmxlIGludGVycnVwdHMpICAgICov
DQo+DQo+IFBsZWFzZSB1c2UgdGhlIG5ldHdvcmsgc3Vic3lzdGVtIGNvbW1lbnQgc3R5bGUuDQo+
DQo+PiArCXN0YXR1cyA9IHR0eS0+ZHJpdmVyLT5vcHMtPnRpb2NtZ2V0KHR0eSk7DQo+PiArCUJU
X0RCRygiJXMgY3VyIHRpb2NtIDB4JXgiLCBfX2Z1bmNfXywgc3RhdHVzKTsNCj4NCj4gCVNwZWxs
IGN1cnJlbnQgb3V0IGhlcmUgYW5kIGdldCByaWQgb2YgX19mdW5jX18NCj4NCj4gSSB3b3VsZCBh
bHNvIGFkZCBhbiBleHRyYSBlbXB0eSBsaW5lIGhlcmUgdG8gbWFrZSBpdCBhIGJpdCBtb3JlIHJl
YWRhYmxlLg0KPg0KPj4gKwlzZXQmPSB+KFRJT0NNX09VVDIgfCBUSU9DTV9SVFMpOw0KPj4gKwlj
bGVhciA9IH5zZXQ7DQo+PiArCXNldCY9IFRJT0NNX0RUUiB8IFRJT0NNX1JUUyB8IFRJT0NNX09V
VDEgfA0KPj4gKwkJVElPQ01fT1VUMiB8IFRJT0NNX0xPT1A7DQo+PiArCWNsZWFyJj0gVElPQ01f
RFRSIHwgVElPQ01fUlRTIHwgVElPQ01fT1VUMSB8DQo+PiArCQlUSU9DTV9PVVQyIHwgVElPQ01f
TE9PUDsNCj4NCj4gWW91IG5lZWQgdG8gYWxpZ24gdGhlIHNlY29uZCBsaW5lcyBjb3JyZWN0bHku
DQo+DQo+PiArCXN0YXR1cyA9IHR0eS0+ZHJpdmVyLT5vcHMtPnRpb2Ntc2V0KHR0eSwgc2V0LCBj
bGVhcik7DQo+PiArCWlmIChzdGF0dXMpDQo+PiArCQlCVF9EQkcoIiVzIGNsciBSVFMgZmFpbCAl
ZCIsIF9fZnVuY19fLCBzdGF0dXMpOw0KPj4gKwllbHNlDQo+PiArCQlCVF9EQkcoIiVzIFJUUyBj
bGVhcmVkIiwgX19mdW5jX18pOw0KPg0KPiBTaW1pbGFyIGNoYW5nZSBhcyBhYm92ZS4NCj4NCj4+
ICsJc3RhdHVzID0gdHR5LT5kcml2ZXItPm9wcy0+dGlvY21nZXQodHR5KTsNCj4NCj4gV2h5IGlz
IHRoaXMgbmVlZGVkPw0KPg0KPj4gK30NCj4+ICsNCj4+ICt2b2lkIGhjaV91YXJ0X3VuZmxvd19j
b250cm9sX2RldmljZShzdHJ1Y3QgaGNpX3VhcnQgKmh1KQ0KPj4gK3sNCj4+ICsJc3RydWN0IHR0
eV9zdHJ1Y3QgKnR0eSA9IGh1LT50dHk7DQo+PiArCXN0cnVjdCBrdGVybWlvcyBrdGVybWlvczsN
Cj4+ICsJaW50IHN0YXR1czsNCj4+ICsJdW5zaWduZWQgaW50IHNldCA9IDA7DQo+PiArCXVuc2ln
bmVkIGludCBjbGVhciA9IDA7DQo+PiArDQo+PiArCXN0YXR1cyA9IHR0eS0+ZHJpdmVyLT5vcHMt
PnRpb2NtZ2V0KHR0eSk7DQo+PiArCUJUX0RCRygiJXMgY3VyIHRpb2NtIDB4JXgiLCBfX2Z1bmNf
Xywgc3RhdHVzKTsNCj4+ICsJc2V0IHw9IChUSU9DTV9PVVQyIHwgVElPQ01fUlRTKTsNCj4+ICsJ
Y2xlYXIgPSB+c2V0Ow0KPj4gKwlzZXQmPSBUSU9DTV9EVFIgfCBUSU9DTV9SVFMgfCBUSU9DTV9P
VVQxIHwNCj4+ICsJCVRJT0NNX09VVDIgfCBUSU9DTV9MT09QOw0KPj4gKwljbGVhciY9IFRJT0NN
X0RUUiB8IFRJT0NNX1JUUyB8IFRJT0NNX09VVDEgfA0KPj4gKwkJVElPQ01fT1VUMiB8IFRJT0NN
X0xPT1A7DQo+PiArCXN0YXR1cyA9IHR0eS0+ZHJpdmVyLT5vcHMtPnRpb2Ntc2V0KHR0eSwgc2V0
LCBjbGVhcik7DQo+PiArCWlmIChzdGF0dXMpDQo+PiArCQlCVF9EQkcoIiVzIHNldCBSVFMgZmFp
bCAlZCIsIF9fZnVuY19fLCBzdGF0dXMpOw0KPj4gKwllbHNlDQo+PiArCQlCVF9EQkcoIiVzIFJU
UyBzZXQiLCBfX2Z1bmNfXyk7DQo+PiArDQo+PiArCS8qIFJlLWVuYWJsZSBoYXJkd2FyZSBmbG93
IGNvbnRyb2wgKi8NCj4+ICsJa3Rlcm1pb3MgPSB0dHktPnRlcm1pb3M7DQo+PiArCWt0ZXJtaW9z
LmNfY2ZsYWcgfD0gQ1JUU0NUUzsNCj4+ICsJc3RhdHVzID0gdHR5X3NldF90ZXJtaW9zKHR0eSwm
a3Rlcm1pb3MpOw0KPj4gKwlpZiAoc3RhdHVzKQ0KPj4gKwkJQlRfREJHKCIlcyBlbmFibGUgZmMg
ZmFpbCAlZCIsIF9fZnVuY19fLCBzdGF0dXMpOw0KPj4gKwllbHNlDQo+PiArCQlCVF9EQkcoIiVz
IGh3IGZjIHJlLWVuYWJsZWQiLCBfX2Z1bmNfXyk7DQo+PiArfQ0KPg0KPiBQcmV0dHkgbXVjaCBz
YW1lIG1vZGlmaWNhdGlvbnMgYXMgZm9yIHRoZSBvdGhlciBmdW5jdGlvbi4NCj4NCj4+ICsNCj4+
ICt2b2lkIGhjaV91YXJ0X3NldF9iYXVkcmF0ZShzdHJ1Y3QgaGNpX3VhcnQgKmh1LCB1bnNpZ25l
ZCBpbnQgc3BlZWQpDQo+PiArew0KPj4gKwlzdHJ1Y3QgdHR5X3N0cnVjdCAqdHR5ID0gaHUtPnR0
eTsNCj4+ICsJc3RydWN0IGt0ZXJtaW9zIGt0ZXJtaW9zOw0KPj4gKw0KPj4gKwkvKiBCcmluZyB0
aGUgVUFSVCBpbnRvIGEga25vd24gc3RhdGUgd2l0aCBhIGdpdmVuIGJhdWQgcmF0ZSAqLw0KPj4g
KwlrdGVybWlvcyA9IHR0eS0+dGVybWlvczsNCj4+ICsJa3Rlcm1pb3MuY19jZmxhZyY9IH5DQkFV
RDsNCj4+ICsJa3Rlcm1pb3MuY19pZmxhZyY9IH4oSUdOQlJLIHwgQlJLSU5UIHwgUEFSTVJLIHwg
SVNUUklQDQo+PiArCQl8IElOTENSIHwgSUdOQ1IgfCBJQ1JOTCB8IElYT04pOw0KPg0KPiBGaXgg
dGhlIGFsaWdubWVudCBoZXJlLg0KPg0KPj4gKwlrdGVybWlvcy5jX29mbGFnJj0gfk9QT1NUOw0K
Pj4gKwlrdGVybWlvcy5jX2xmbGFnJj0gfihFQ0hPIHwgRUNIT05MIHwgSUNBTk9OIHwgSVNJRyB8
IElFWFRFTik7DQo+PiArCWt0ZXJtaW9zLmNfY2ZsYWcmPSB+KENTSVpFIHwgUEFSRU5CIHwgQ0JB
VUQpOw0KPj4gKwlrdGVybWlvcy5jX2NmbGFnIHw9IENTODsNCj4+ICsJa3Rlcm1pb3MuY19jZmxh
ZyB8PSBDUlRTQ1RTOw0KPj4gKwkvKiBrdGVybWlvcy5jX2NmbGFnIHw9IEJPVEhFUjsgKi8NCj4N
Cj4gRG9u4oCZdCBsaWtlIHVuY29tbWVudGVkIGNvZGUuIEJldHRlciB0byBqdXN0IHJlbW92ZSBp
dC4NCj4NCj4+ICsJdHR5X3Rlcm1pb3NfZW5jb2RlX2JhdWRfcmF0ZSgma3Rlcm1pb3MsIHNwZWVk
LCBzcGVlZCk7DQo+PiArDQo+PiArCS8qIHR0eV9zZXRfdGVybWlvcygpIHJldHVybiBub3QgY2hl
Y2tlZCBhcyBpdCBpcyBhbHdheXMgMCAqLw0KPj4gKwl0dHlfc2V0X3Rlcm1pb3ModHR5LCZrdGVy
bWlvcyk7DQo+PiArDQo+PiArCUJUX0RCRygiJXM6IE5ldyB0dHkgc3BlZWRzOiAlZC8lZCwgY2Zs
YWc6IDB4JXgiLCBodS0+aGRldi0+bmFtZSwNCj4+ICsJICAgICAgIHR0eS0+dGVybWlvcy5jX2lz
cGVlZCwgdHR5LT50ZXJtaW9zLmNfb3NwZWVkLA0KPj4gKwkgICAgICAgdHR5LT50ZXJtaW9zLmNf
Y2ZsYWcpOw0KPj4gK30NCj4+ICsNCj4+IHN0YXRpYyBpbnQgaGNpX3VhcnRfc2V0dXAoc3RydWN0
IGhjaV9kZXYgKmhkZXYpDQo+PiB7DQo+PiAJc3RydWN0IGhjaV91YXJ0ICpodSA9IGhjaV9nZXRf
ZHJ2ZGF0YShoZGV2KTsNCj4+IAlzdHJ1Y3QgaGNpX3JwX3JlYWRfbG9jYWxfdmVyc2lvbiAqdmVy
Ow0KPj4gCXN0cnVjdCBza19idWZmICpza2I7DQo+PiArCWludCBlcnI7DQo+PiArDQo+PiArCWlm
IChodS0+cHJvdG8tPmluaXRfc3BlZWQpDQo+PiArCQloY2lfdWFydF9zZXRfYmF1ZHJhdGUoaHUs
IGh1LT5wcm90by0+aW5pdF9zcGVlZCk7DQo+PiArDQo+PiArCWlmIChodS0+cHJvdG8tPnNldF9i
YXVkcmF0ZSYmICBodS0+cHJvdG8tPm9wZXJfc3BlZWQpIHsNCj4+ICsJCWVyciA9IGh1LT5wcm90
by0+c2V0X2JhdWRyYXRlKGh1LCBodS0+cHJvdG8tPm9wZXJfc3BlZWQpOw0KPj4gKwkJaWYgKCFl
cnIpDQo+PiArCQkJaGNpX3VhcnRfc2V0X2JhdWRyYXRlKGh1LCBodS0+cHJvdG8tPm9wZXJfc3Bl
ZWQpOw0KPj4gKwl9DQo+Pg0KPj4gCWlmIChodS0+cHJvdG8tPnNldHVwKQ0KPj4gCQlyZXR1cm4g
aHUtPnByb3RvLT5zZXR1cChodSk7DQo+PiBAQCAtNjQ3LDcgKzc1MSw3IEBAIHN0YXRpYyBpbnQg
X19pbml0IGhjaV91YXJ0X2luaXQodm9pZCkNCj4+DQo+PiAJLyogUmVnaXN0ZXIgdGhlIHR0eSBk
aXNjaXBsaW5lICovDQo+Pg0KPj4gLQltZW1zZXQoJmhjaV91YXJ0X2xkaXNjLCAwLCBzaXplb2Yg
KGhjaV91YXJ0X2xkaXNjKSk7DQo+PiArCW1lbXNldCgmaGNpX3VhcnRfbGRpc2MsIDAsIHNpemVv
ZihoY2lfdWFydF9sZGlzYykpOw0KPg0KPiBJZiB0aGlzIGlzIHdyb25nIGluIGV4aXN0aW5nIGNv
ZGUsIHBsZWFzZSBzZW5kIGEgY2xlYW51cCBwYXRjaCBmb3IgaXQuIEVhc3kgdG8gYXBwbHkuDQo+
DQo+PiAJaGNpX3VhcnRfbGRpc2MubWFnaWMJCT0gVFRZX0xESVNDX01BR0lDOw0KPj4gCWhjaV91
YXJ0X2xkaXNjLm5hbWUJCT0gIm5faGNpIjsNCj4+IAloY2lfdWFydF9sZGlzYy5vcGVuCQk9IGhj
aV91YXJ0X3R0eV9vcGVuOw0KPj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvYmx1ZXRvb3RoL2hjaV91
YXJ0LmggYi9kcml2ZXJzL2JsdWV0b290aC9oY2lfdWFydC5oDQo+PiBpbmRleCA3MjEyMGE1Li4y
MjcxY2MwIDEwMDY0NA0KPj4gLS0tIGEvZHJpdmVycy9ibHVldG9vdGgvaGNpX3VhcnQuaA0KPj4g
KysrIGIvZHJpdmVycy9ibHVldG9vdGgvaGNpX3VhcnQuaA0KPj4gQEAgLTU4LDEwICs1OCwxMyBA
QCBzdHJ1Y3QgaGNpX3VhcnQ7DQo+PiBzdHJ1Y3QgaGNpX3VhcnRfcHJvdG8gew0KPj4gCXVuc2ln
bmVkIGludCBpZDsNCj4+IAljb25zdCBjaGFyICpuYW1lOw0KPj4gKwl1bnNpZ25lZCBpbnQgaW5p
dF9zcGVlZDsNCj4+ICsJdW5zaWduZWQgaW50IG9wZXJfc3BlZWQ7DQo+PiAJaW50ICgqb3Blbiko
c3RydWN0IGhjaV91YXJ0ICpodSk7DQo+PiAJaW50ICgqY2xvc2UpKHN0cnVjdCBoY2lfdWFydCAq
aHUpOw0KPj4gCWludCAoKmZsdXNoKShzdHJ1Y3QgaGNpX3VhcnQgKmh1KTsNCj4+IAlpbnQgKCpz
ZXR1cCkoc3RydWN0IGhjaV91YXJ0ICpodSk7DQo+PiArCWludCAoKnNldF9iYXVkcmF0ZSkoc3Ry
dWN0IGhjaV91YXJ0ICpodSwgdW5zaWduZWQgaW50IHNwZWVkKTsNCj4+IAlpbnQgKCpyZWN2KShz
dHJ1Y3QgaGNpX3VhcnQgKmh1LCBjb25zdCB2b2lkICpkYXRhLCBpbnQgbGVuKTsNCj4+IAlpbnQg
KCplbnF1ZXVlKShzdHJ1Y3QgaGNpX3VhcnQgKmh1LCBzdHJ1Y3Qgc2tfYnVmZiAqc2tiKTsNCj4+
IAlzdHJ1Y3Qgc2tfYnVmZiAqKCpkZXF1ZXVlKShzdHJ1Y3QgaGNpX3VhcnQgKmh1KTsNCj4+IEBA
IC05Niw2ICs5OSw5IEBAIGludCBoY2lfdWFydF9yZWdpc3Rlcl9wcm90byhjb25zdCBzdHJ1Y3Qg
aGNpX3VhcnRfcHJvdG8gKnApOw0KPj4gaW50IGhjaV91YXJ0X3VucmVnaXN0ZXJfcHJvdG8oY29u
c3Qgc3RydWN0IGhjaV91YXJ0X3Byb3RvICpwKTsNCj4+IGludCBoY2lfdWFydF90eF93YWtldXAo
c3RydWN0IGhjaV91YXJ0ICpodSk7DQo+PiBpbnQgaGNpX3VhcnRfaW5pdF9yZWFkeShzdHJ1Y3Qg
aGNpX3VhcnQgKmh1KTsNCj4+ICt2b2lkIGhjaV91YXJ0X3NldF9iYXVkcmF0ZShzdHJ1Y3QgaGNp
X3VhcnQgKmh1LCB1bnNpZ25lZCBpbnQgc3BlZWQpOw0KPj4gK3ZvaWQgaGNpX3VhcnRfZmxvd19j
b250cm9sX2RldmljZShzdHJ1Y3QgaGNpX3VhcnQgKmh1KTsNCj4+ICt2b2lkIGhjaV91YXJ0X3Vu
Zmxvd19jb250cm9sX2RldmljZShzdHJ1Y3QgaGNpX3VhcnQgKmh1KTsNCj4NCj4gSSBhY3R1YWxs
eSBkaXNsaWtlIHRoaXMga2luZCBvZiBuYW1pbmcuIFdoYXQgYWJvdXQgc2ltcGxlIHN0dWZmIGxp
a2U6DQo+DQo+IAloY2lfdWFydF9lbmFibGVfZGV2aWNlKCkNCj4gCWhjaV91YXJ0X2Rpc2FibGVf
ZGV2aWNlKCkNCj4NCj4gT3Igc29tZXRoaW5nIHdpdGggYSBib29sZWFuIGZvciB0aGUgc3RhdGU6
DQo+DQo+IAloY2lfdWFydF9zZXRfZmxvd19jb250cm9sKHN0cnVjdCBoY2lfdWFydCAqaHUsIGJv
b2wgZW5hYmxlKQ0KPg0KPiBSZWdhcmRzDQo+DQo+IE1hcmNlbA0KPg0KDQo=

2015-06-06 17:48:00

by Peter Hurley

[permalink] [raw]
Subject: Re: [PATCH 2/5] Intel based H4 line discipline enhancements

Hi Marcel,

On 06/06/2015 12:39 PM, Marcel Holtmann wrote:
> However sparse run happens only after I already have the patch applied. If something goes wrong there, that is really wasting my time. And 0day testing bot will find it eventually as well.

FWIW, Greg uses a testing branch that the 0-day bot runs, and only later
moves those commits to -next.

Not that contributors shouldn't be using make C=2.

Regards,
Peter Hurley

2015-06-06 16:39:10

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 2/5] Intel based H4 line discipline enhancements

Hi Arend,

>> Appreciate you applying Fred's patches. I will now put together and post similar patches against the bluetooth-next proper.
>>
>> Sorry for many minor changes. I just happen to run the checkpatch script automatically prior to publishing the patches (Arend taught me good Linux practices :-)) and that results in those spaces getting deleted et al.
>
> Actually, checkpatch by default only does the check and just tell you what is wrong. Normally, it would only warn about the thing your patch modified. Unless you ran checkpatch on source file.

personally it is even more important that people run make C=2 drivers/bluetooth/ to run the sparse and endian checks. Most of the coding style issues, I can spot in the email client before applying the patch. However sparse run happens only after I already have the patch applied. If something goes wrong there, that is really wasting my time. And 0day testing bot will find it eventually as well.

Regards

Marcel


2015-06-06 16:03:01

by chanyeol

[permalink] [raw]
Subject: Re: [PATCH 4/5] Broadcom Bluetooth protocol UART support

Hi Arend,

On Sat, 2015-06-06 at 10:31 +0200, Marcel Holtmann wrote:
> Hi Arend,
>
> > > > Merge of the Broadcom logic into the Intel's btbcm
> > > > implementation.
> > > >
> > > > Signed-off-by: Ilya Faenson<[email protected]>
> > > > ---
> > > > drivers/bluetooth/btbcm.c | 142
> > > > ++++++++++++++++++++++++++++++++++++----------
> > > > drivers/bluetooth/btbcm.h | 21 ++++++-
> > > > 2 files changed, 132 insertions(+), 31 deletions(-)
> > > >
> > > > diff --git a/drivers/bluetooth/btbcm.c
> > > > b/drivers/bluetooth/btbcm.c
> > > > index 728fce3..e1d5ad0 100644
> > > > --- a/drivers/bluetooth/btbcm.c
> > > > +++ b/drivers/bluetooth/btbcm.c
> > > > @@ -3,6 +3,7 @@
> > > > * Bluetooth support for Broadcom devices
> > > > *
> > > > * Copyright (C) 2015 Intel Corporation
> > > > + * Copyright (C) 2015 Broadcom Corporation
> > > > *
> > > > *
> > > > * This program is free software; you can redistribute it
> > > > and/or modify
> > > > @@ -32,7 +33,8 @@
> > > >
> > > > #define VERSION "0.1"
> > > >
> > > > -#define BDADDR_BCM20702A0 (&(bdaddr_t) {{0x00, 0xa0, 0x02,
> > > > 0x70, 0x20, 0x00}})
> > > > +#define BDADDR_BCM20702A0 (&(bdaddr_t) {{0x00, 0xa0, 0x02,
> > > > 0x70, 0x20, 0x00} })
> > > > +#define BDADDR_BCM4324B3 (&(bdaddr_t) {{0x00, 0x00, 0x00,
> > > > 0xb3, 0x24, 0x43} })
> > >
> > > something funky is going on with your editor that insist on
> > > fixing this ;)
> > >
> > > >
> > > > int btbcm_check_bdaddr(struct hci_dev *hdev)
> > > > {
> > > > @@ -43,6 +45,7 @@ int btbcm_check_bdaddr(struct hci_dev *hdev)
> > > > HCI_INIT_TIMEOUT);
> > > > if (IS_ERR(skb)) {
> > > > int err = PTR_ERR(skb);
> > > > +
> > >
> > > In this specific case, please do not fix it. I like to keep the
> > > simple error path sections small.
> > >
> > > > BT_ERR("%s: BCM: Reading device address failed
> > > > (%d)",
> > > > hdev->name, err);
> > > > return err;
> > > > @@ -56,10 +59,11 @@ int btbcm_check_bdaddr(struct hci_dev
> > > > *hdev)
> > > >
> > > > bda = (struct hci_rp_read_bd_addr *)skb->data;
> > > >
> > > > - /* The address 00:20:70:02:A0:00 indicates a
> > > > BCM20702A0 controller
> > > > - * with no configured address.
> > > > + /* Check if the address indicates a controller with no
> > > > configured
> > > > + * address.
> > > > */
> > > > - if (!bacmp(&bda->bdaddr, BDADDR_BCM20702A0)) {
> > > > + if (!bacmp(&bda->bdaddr, BDADDR_BCM20702A0) ||
> > > > + !bacmp(&bda->bdaddr, BDADDR_BCM4324B3)) {
> > > > BT_INFO("%s: BCM: Using default device address
> > > > (%pMR)",
> > > > hdev->name,&bda->bdaddr);
> > > > set_bit(HCI_QUIRK_INVALID_BDADDR,&hdev
> > > > ->quirks);
> > > > @@ -89,21 +93,14 @@ int btbcm_set_bdaddr(struct hci_dev *hdev,
> > > > const bdaddr_t *bdaddr)
> > > > }
> > > > EXPORT_SYMBOL_GPL(btbcm_set_bdaddr);
> > > >
> > > > -int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
> > > > +int btbcm_patchram(struct hci_dev *hdev, const struct firmware
> > > > *fw)
> > > > {
> > > > const struct hci_command_hdr *cmd;
> > > > - const struct firmware *fw;
> > > > const u8 *fw_ptr;
> > > > size_t fw_size;
> > > > struct sk_buff *skb;
> > > > u16 opcode;
> > > > - int err;
> > > > -
> > > > - err = request_firmware(&fw, firmware,&hdev->dev);
> > > > - if (err< 0) {
> > > > - BT_INFO("%s: BCM: Patch %s not found", hdev
> > > > ->name, firmware);
> > > > - return err;
> > > > - }
> > > > + int err = 0;
> > > >
> > > > /* Start Download */
> > > > skb = __hci_cmd_sync(hdev, 0xfc2e, 0, NULL,
> > > > HCI_INIT_TIMEOUT);
> > > > @@ -129,8 +126,7 @@ int btbcm_patchram(struct hci_dev *hdev,
> > > > const char *firmware)
> > > > fw_size -= sizeof(*cmd);
> > > >
> > > > if (fw_size< cmd->plen) {
> > > > - BT_ERR("%s: BCM: Patch %s is
> > > > corrupted", hdev->name,
> > > > - firmware);
> > > > + BT_ERR("%s: BCM: Patch is corrupted",
> > > > hdev->name);
> > > > err = -EINVAL;
> > > > goto done;
> > > > }
> > > > @@ -156,7 +152,6 @@ int btbcm_patchram(struct hci_dev *hdev,
> > > > const char *firmware)
> > > > msleep(250);
> > > >
> > > > done:
> > > > - release_firmware(fw);
> > > > return err;
> > > > }
> > > > EXPORT_SYMBOL(btbcm_patchram);
> > > > @@ -168,6 +163,7 @@ static int btbcm_reset(struct hci_dev
> > > > *hdev)
> > > > skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL,
> > > > HCI_INIT_TIMEOUT);
> > > > if (IS_ERR(skb)) {
> > > > int err = PTR_ERR(skb);
> > > > +
> > > > BT_ERR("%s: BCM: Reset failed (%d)", hdev
> > > > ->name, err);
> > > > return err;
> > > > }
> > > > @@ -242,9 +238,101 @@ static const struct {
> > > > const char *name;
> > > > } bcm_uart_subver_table[] = {
> > > > { 0x410e, "BCM43341B0" }, /* 002.001.014 */
> > > > + { 0x4406, "BCM4324B3" }, /* 002.004.006 */
> > > > + { 0x610c, "BCM4354_003.001.012.0306.0659"}, /*
> > > > 003.001.012 */
> > >
> > > Please just BCM4354 here.
> > >
> > > And I have initial patches for the manifest file to map modalias
> > > to firmware names. That will then hopefully solve all of these
> > > and we can just update this easily from userspace. I will send
> > > patches as soon as I have cleaned them up a bit.
> >
> > Is using modalias sufficient. At least for our wifi chips it is not
> > as new chip revisions with same modalias require different
> > firmware. Same may be true for bt chips.
>
> let me put it this way, the Windows driver maps the firmware based on
> USB VID/PID and thus that seems fine then. Also it seems that every
> ACPI based platform gets a proper ID assigned. So that makes sense as
> well there.
>
> So there are plenty of USB dongles that share the same firmware, but
> it seems to be manual mapping based on who tested and certified a
> given firmware for a given piece of hardware. It is just too many
> that I really want to know. I rather have them all listed and then
> someone can update it from userspace instead of having to deal with
> it in the kernel.
Also I think Kernel might not have enough info to get the exact
firmware file name. So userspace needs to help kernel to get the exact
firmware.

IMO I guess match would happen in hciattach and it could be updated on
kernel node.

Thanks
Chanyeol
>
> Regards
>
> Marcel
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux
> -bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2015-06-06 15:50:31

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 2/5] Intel based H4 line discipline enhancements

Hi Ilya,

the mailing list has a strict policy of no top-posting. So please adhere to that. I know it is hard with corporate email systems, but for mailing list emails I am policing this.

> Appreciate you applying Fred's patches. I will now put together and post similar patches against the bluetooth-next proper.
>
> Sorry for many minor changes. I just happen to run the checkpatch script automatically prior to publishing the patches (Arend taught me good Linux practices :-)) and that results in those spaces getting deleted et al.

Please keep in that you need --strict actually. We have to adhere to the network subsystem rules. Also some of the driver code is pretty old. You will find violations. Feel free to fix them, but please do it in a separate patch before your changes.

Regards

Marcel


2015-06-06 15:40:33

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH 2/5] Intel based H4 line discipline enhancements

On 06/06/15 17:33, Ilya Faenson wrote:
> Hi Marcel,
>
> Appreciate you applying Fred's patches. I will now put together and post similar patches against the bluetooth-next proper.
>
> Sorry for many minor changes. I just happen to run the checkpatch script automatically prior to publishing the patches (Arend taught me good Linux practices :-)) and that results in those spaces getting deleted et al.

Actually, checkpatch by default only does the check and just tell you
what is wrong. Normally, it would only warn about the thing your patch
modified. Unless you ran checkpatch on source file.

Regards,
Arend

> Thanks,
> -Ilya
>
> -----Original Message-----
> From: Marcel Holtmann [mailto:[email protected]]
> Sent: Saturday, June 06, 2015 2:37 AM
> To: Ilya Faenson
> Cc: [email protected]
> Subject: Re: [PATCH 2/5] Intel based H4 line discipline enhancements
>
> Hi Ilya,
>
>> This is largely enhancements implemented by Frederic Danis of Intel.
>> I've also added the ability to flow control the UART and improved the
>> UART baud rate setting some.
>
> I applied parts of Fred’s patches and so please rebase on top of them. Makes it a lot easier for me to review changes instead of figuring out what comes from you and what comes from Fred.
>
>>
>> Signed-off-by: Ilya Faenson<[email protected]>
>> ---
>> drivers/bluetooth/hci_ldisc.c | 110 ++++++++++++++++++++++++++++++++++++++++--
>> drivers/bluetooth/hci_uart.h | 6 +++
>> 2 files changed, 113 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
>> index 5c9a73f..58dcb24 100644
>> --- a/drivers/bluetooth/hci_ldisc.c
>> +++ b/drivers/bluetooth/hci_ldisc.c
>> @@ -1,4 +1,4 @@
>> -/*
>> +/*_
>
> Funky change ;)
>
>> *
>> * Bluetooth HCI UART driver
>> *
>> @@ -256,7 +256,8 @@ static int hci_uart_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
>> if (!test_bit(HCI_RUNNING,&hdev->flags))
>> return -EBUSY;
>>
>> - BT_DBG("%s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type, skb->len);
>> + BT_DBG("%s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type,
>> + skb->len);
>
> In case you spot coding style errors, please just send them as tiny cleanup patches. I can easily apply them out of order and we get of this. Makes review a lot easier.
>
>>
>> hu->proto->enqueue(hu, skb);
>>
>> @@ -265,11 +266,114 @@ static int hci_uart_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
>> return 0;
>> }
>>
>> +void hci_uart_flow_control_device(struct hci_uart *hu)
>> +{
>> + struct tty_struct *tty = hu->tty;
>> + struct ktermios ktermios;
>> + int status;
>> + unsigned int set = 0;
>> + unsigned int clear = 0;
>> +
>> + /* Disable hardware flow control */
>> + ktermios = tty->termios;
>> + ktermios.c_cflag&= ~CRTSCTS;
>> + status = tty_set_termios(tty,&ktermios);
>> + if (status)
>> + BT_DBG("%s dis fc failure %d", __func__, status);
>> + else
>> + BT_DBG("%s hw fc disabled", __func__);
>
> Lets do it like this:
>
> BT_DBG(“Disabling hardware flow control: %s”, status ? “failed” : “success”);
>
>> +
>> + /* Clear RTS to prevent the device from sending */
>> + /* (most PCs need OUT2 to enable interrupts) */
>
> Please use the network subsystem comment style.
>
>> + status = tty->driver->ops->tiocmget(tty);
>> + BT_DBG("%s cur tiocm 0x%x", __func__, status);
>
> Spell current out here and get rid of __func__
>
> I would also add an extra empty line here to make it a bit more readable.
>
>> + set&= ~(TIOCM_OUT2 | TIOCM_RTS);
>> + clear = ~set;
>> + set&= TIOCM_DTR | TIOCM_RTS | TIOCM_OUT1 |
>> + TIOCM_OUT2 | TIOCM_LOOP;
>> + clear&= TIOCM_DTR | TIOCM_RTS | TIOCM_OUT1 |
>> + TIOCM_OUT2 | TIOCM_LOOP;
>
> You need to align the second lines correctly.
>
>> + status = tty->driver->ops->tiocmset(tty, set, clear);
>> + if (status)
>> + BT_DBG("%s clr RTS fail %d", __func__, status);
>> + else
>> + BT_DBG("%s RTS cleared", __func__);
>
> Similar change as above.
>
>> + status = tty->driver->ops->tiocmget(tty);
>
> Why is this needed?
>
>> +}
>> +
>> +void hci_uart_unflow_control_device(struct hci_uart *hu)
>> +{
>> + struct tty_struct *tty = hu->tty;
>> + struct ktermios ktermios;
>> + int status;
>> + unsigned int set = 0;
>> + unsigned int clear = 0;
>> +
>> + status = tty->driver->ops->tiocmget(tty);
>> + BT_DBG("%s cur tiocm 0x%x", __func__, status);
>> + set |= (TIOCM_OUT2 | TIOCM_RTS);
>> + clear = ~set;
>> + set&= TIOCM_DTR | TIOCM_RTS | TIOCM_OUT1 |
>> + TIOCM_OUT2 | TIOCM_LOOP;
>> + clear&= TIOCM_DTR | TIOCM_RTS | TIOCM_OUT1 |
>> + TIOCM_OUT2 | TIOCM_LOOP;
>> + status = tty->driver->ops->tiocmset(tty, set, clear);
>> + if (status)
>> + BT_DBG("%s set RTS fail %d", __func__, status);
>> + else
>> + BT_DBG("%s RTS set", __func__);
>> +
>> + /* Re-enable hardware flow control */
>> + ktermios = tty->termios;
>> + ktermios.c_cflag |= CRTSCTS;
>> + status = tty_set_termios(tty,&ktermios);
>> + if (status)
>> + BT_DBG("%s enable fc fail %d", __func__, status);
>> + else
>> + BT_DBG("%s hw fc re-enabled", __func__);
>> +}
>
> Pretty much same modifications as for the other function.
>
>> +
>> +void hci_uart_set_baudrate(struct hci_uart *hu, unsigned int speed)
>> +{
>> + struct tty_struct *tty = hu->tty;
>> + struct ktermios ktermios;
>> +
>> + /* Bring the UART into a known state with a given baud rate */
>> + ktermios = tty->termios;
>> + ktermios.c_cflag&= ~CBAUD;
>> + ktermios.c_iflag&= ~(IGNBRK | BRKINT | PARMRK | ISTRIP
>> + | INLCR | IGNCR | ICRNL | IXON);
>
> Fix the alignment here.
>
>> + ktermios.c_oflag&= ~OPOST;
>> + ktermios.c_lflag&= ~(ECHO | ECHONL | ICANON | ISIG | IEXTEN);
>> + ktermios.c_cflag&= ~(CSIZE | PARENB | CBAUD);
>> + ktermios.c_cflag |= CS8;
>> + ktermios.c_cflag |= CRTSCTS;
>> + /* ktermios.c_cflag |= BOTHER; */
>
> Don’t like uncommented code. Better to just remove it.
>
>> + tty_termios_encode_baud_rate(&ktermios, speed, speed);
>> +
>> + /* tty_set_termios() return not checked as it is always 0 */
>> + tty_set_termios(tty,&ktermios);
>> +
>> + BT_DBG("%s: New tty speeds: %d/%d, cflag: 0x%x", hu->hdev->name,
>> + tty->termios.c_ispeed, tty->termios.c_ospeed,
>> + tty->termios.c_cflag);
>> +}
>> +
>> static int hci_uart_setup(struct hci_dev *hdev)
>> {
>> struct hci_uart *hu = hci_get_drvdata(hdev);
>> struct hci_rp_read_local_version *ver;
>> struct sk_buff *skb;
>> + int err;
>> +
>> + if (hu->proto->init_speed)
>> + hci_uart_set_baudrate(hu, hu->proto->init_speed);
>> +
>> + if (hu->proto->set_baudrate&& hu->proto->oper_speed) {
>> + err = hu->proto->set_baudrate(hu, hu->proto->oper_speed);
>> + if (!err)
>> + hci_uart_set_baudrate(hu, hu->proto->oper_speed);
>> + }
>>
>> if (hu->proto->setup)
>> return hu->proto->setup(hu);
>> @@ -647,7 +751,7 @@ static int __init hci_uart_init(void)
>>
>> /* Register the tty discipline */
>>
>> - memset(&hci_uart_ldisc, 0, sizeof (hci_uart_ldisc));
>> + memset(&hci_uart_ldisc, 0, sizeof(hci_uart_ldisc));
>
> If this is wrong in existing code, please send a cleanup patch for it. Easy to apply.
>
>> hci_uart_ldisc.magic = TTY_LDISC_MAGIC;
>> hci_uart_ldisc.name = "n_hci";
>> hci_uart_ldisc.open = hci_uart_tty_open;
>> diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
>> index 72120a5..2271cc0 100644
>> --- a/drivers/bluetooth/hci_uart.h
>> +++ b/drivers/bluetooth/hci_uart.h
>> @@ -58,10 +58,13 @@ struct hci_uart;
>> struct hci_uart_proto {
>> unsigned int id;
>> const char *name;
>> + unsigned int init_speed;
>> + unsigned int oper_speed;
>> int (*open)(struct hci_uart *hu);
>> int (*close)(struct hci_uart *hu);
>> int (*flush)(struct hci_uart *hu);
>> int (*setup)(struct hci_uart *hu);
>> + int (*set_baudrate)(struct hci_uart *hu, unsigned int speed);
>> int (*recv)(struct hci_uart *hu, const void *data, int len);
>> int (*enqueue)(struct hci_uart *hu, struct sk_buff *skb);
>> struct sk_buff *(*dequeue)(struct hci_uart *hu);
>> @@ -96,6 +99,9 @@ int hci_uart_register_proto(const struct hci_uart_proto *p);
>> int hci_uart_unregister_proto(const struct hci_uart_proto *p);
>> int hci_uart_tx_wakeup(struct hci_uart *hu);
>> int hci_uart_init_ready(struct hci_uart *hu);
>> +void hci_uart_set_baudrate(struct hci_uart *hu, unsigned int speed);
>> +void hci_uart_flow_control_device(struct hci_uart *hu);
>> +void hci_uart_unflow_control_device(struct hci_uart *hu);
>
> I actually dislike this kind of naming. What about simple stuff like:
>
> hci_uart_enable_device()
> hci_uart_disable_device()
>
> Or something with a boolean for the state:
>
> hci_uart_set_flow_control(struct hci_uart *hu, bool enable)
>
> Regards
>
> Marcel
>

2015-06-06 15:33:16

by Ilya Faenson

[permalink] [raw]
Subject: RE: [PATCH 2/5] Intel based H4 line discipline enhancements

SGkgTWFyY2VsLA0KDQpBcHByZWNpYXRlIHlvdSBhcHBseWluZyBGcmVkJ3MgcGF0Y2hlcy4gSSB3
aWxsIG5vdyBwdXQgdG9nZXRoZXIgYW5kIHBvc3Qgc2ltaWxhciBwYXRjaGVzIGFnYWluc3QgdGhl
IGJsdWV0b290aC1uZXh0IHByb3Blci4NCg0KU29ycnkgZm9yIG1hbnkgbWlub3IgY2hhbmdlcy4g
SSBqdXN0IGhhcHBlbiB0byBydW4gdGhlIGNoZWNrcGF0Y2ggc2NyaXB0IGF1dG9tYXRpY2FsbHkg
cHJpb3IgdG8gcHVibGlzaGluZyB0aGUgcGF0Y2hlcyAoQXJlbmQgdGF1Z2h0IG1lIGdvb2QgTGlu
dXggcHJhY3RpY2VzIDotKSkgYW5kIHRoYXQgcmVzdWx0cyBpbiB0aG9zZSBzcGFjZXMgZ2V0dGlu
ZyBkZWxldGVkIGV0IGFsLg0KDQpUaGFua3MsDQogLUlseWENCg0KLS0tLS1PcmlnaW5hbCBNZXNz
YWdlLS0tLS0NCkZyb206IE1hcmNlbCBIb2x0bWFubiBbbWFpbHRvOm1hcmNlbEBob2x0bWFubi5v
cmddIA0KU2VudDogU2F0dXJkYXksIEp1bmUgMDYsIDIwMTUgMjozNyBBTQ0KVG86IElseWEgRmFl
bnNvbg0KQ2M6IGxpbnV4LWJsdWV0b290aEB2Z2VyLmtlcm5lbC5vcmcNClN1YmplY3Q6IFJlOiBb
UEFUQ0ggMi81XSBJbnRlbCBiYXNlZCBINCBsaW5lIGRpc2NpcGxpbmUgZW5oYW5jZW1lbnRzDQoN
CkhpIElseWEsDQoNCj4gVGhpcyBpcyBsYXJnZWx5IGVuaGFuY2VtZW50cyBpbXBsZW1lbnRlZCBi
eSBGcmVkZXJpYyBEYW5pcyBvZiBJbnRlbC4NCj4gSSd2ZSBhbHNvIGFkZGVkIHRoZSBhYmlsaXR5
IHRvIGZsb3cgY29udHJvbCB0aGUgVUFSVCBhbmQgaW1wcm92ZWQgdGhlDQo+IFVBUlQgYmF1ZCBy
YXRlIHNldHRpbmcgc29tZS4NCg0KSSBhcHBsaWVkIHBhcnRzIG9mIEZyZWTigJlzIHBhdGNoZXMg
YW5kIHNvIHBsZWFzZSByZWJhc2Ugb24gdG9wIG9mIHRoZW0uIE1ha2VzIGl0IGEgbG90IGVhc2ll
ciBmb3IgbWUgdG8gcmV2aWV3IGNoYW5nZXMgaW5zdGVhZCBvZiBmaWd1cmluZyBvdXQgd2hhdCBj
b21lcyBmcm9tIHlvdSBhbmQgd2hhdCBjb21lcyBmcm9tIEZyZWQuDQoNCj4gDQo+IFNpZ25lZC1v
ZmYtYnk6IElseWEgRmFlbnNvbiA8aWZhZW5zb25AYnJvYWRjb20uY29tPg0KPiAtLS0NCj4gZHJp
dmVycy9ibHVldG9vdGgvaGNpX2xkaXNjLmMgfCAxMTAgKysrKysrKysrKysrKysrKysrKysrKysr
KysrKysrKysrKysrKysrKy0tDQo+IGRyaXZlcnMvYmx1ZXRvb3RoL2hjaV91YXJ0LmggIHwgICA2
ICsrKw0KPiAyIGZpbGVzIGNoYW5nZWQsIDExMyBpbnNlcnRpb25zKCspLCAzIGRlbGV0aW9ucygt
KQ0KPiANCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvYmx1ZXRvb3RoL2hjaV9sZGlzYy5jIGIvZHJp
dmVycy9ibHVldG9vdGgvaGNpX2xkaXNjLmMNCj4gaW5kZXggNWM5YTczZi4uNThkY2IyNCAxMDA2
NDQNCj4gLS0tIGEvZHJpdmVycy9ibHVldG9vdGgvaGNpX2xkaXNjLmMNCj4gKysrIGIvZHJpdmVy
cy9ibHVldG9vdGgvaGNpX2xkaXNjLmMNCj4gQEAgLTEsNCArMSw0IEBADQo+IC0vKg0KPiArLypf
DQoNCkZ1bmt5IGNoYW5nZSA7KQ0KDQo+ICAqDQo+ICAqICBCbHVldG9vdGggSENJIFVBUlQgZHJp
dmVyDQo+ICAqDQo+IEBAIC0yNTYsNyArMjU2LDggQEAgc3RhdGljIGludCBoY2lfdWFydF9zZW5k
X2ZyYW1lKHN0cnVjdCBoY2lfZGV2ICpoZGV2LCBzdHJ1Y3Qgc2tfYnVmZiAqc2tiKQ0KPiAJaWYg
KCF0ZXN0X2JpdChIQ0lfUlVOTklORywgJmhkZXYtPmZsYWdzKSkNCj4gCQlyZXR1cm4gLUVCVVNZ
Ow0KPiANCj4gLQlCVF9EQkcoIiVzOiB0eXBlICVkIGxlbiAlZCIsIGhkZXYtPm5hbWUsIGJ0X2Ni
KHNrYiktPnBrdF90eXBlLCBza2ItPmxlbik7DQo+ICsJQlRfREJHKCIlczogdHlwZSAlZCBsZW4g
JWQiLCBoZGV2LT5uYW1lLCBidF9jYihza2IpLT5wa3RfdHlwZSwNCj4gKwkgICAgICAgc2tiLT5s
ZW4pOw0KDQpJbiBjYXNlIHlvdSBzcG90IGNvZGluZyBzdHlsZSBlcnJvcnMsIHBsZWFzZSBqdXN0
IHNlbmQgdGhlbSBhcyB0aW55IGNsZWFudXAgcGF0Y2hlcy4gSSBjYW4gZWFzaWx5IGFwcGx5IHRo
ZW0gb3V0IG9mIG9yZGVyIGFuZCB3ZSBnZXQgb2YgdGhpcy4gTWFrZXMgcmV2aWV3IGEgbG90IGVh
c2llci4NCg0KPiANCj4gCWh1LT5wcm90by0+ZW5xdWV1ZShodSwgc2tiKTsNCj4gDQo+IEBAIC0y
NjUsMTEgKzI2NiwxMTQgQEAgc3RhdGljIGludCBoY2lfdWFydF9zZW5kX2ZyYW1lKHN0cnVjdCBo
Y2lfZGV2ICpoZGV2LCBzdHJ1Y3Qgc2tfYnVmZiAqc2tiKQ0KPiAJcmV0dXJuIDA7DQo+IH0NCj4g
DQo+ICt2b2lkIGhjaV91YXJ0X2Zsb3dfY29udHJvbF9kZXZpY2Uoc3RydWN0IGhjaV91YXJ0ICpo
dSkNCj4gK3sNCj4gKwlzdHJ1Y3QgdHR5X3N0cnVjdCAqdHR5ID0gaHUtPnR0eTsNCj4gKwlzdHJ1
Y3Qga3Rlcm1pb3Mga3Rlcm1pb3M7DQo+ICsJaW50IHN0YXR1czsNCj4gKwl1bnNpZ25lZCBpbnQg
c2V0ID0gMDsNCj4gKwl1bnNpZ25lZCBpbnQgY2xlYXIgPSAwOw0KPiArDQo+ICsJLyogRGlzYWJs
ZSBoYXJkd2FyZSBmbG93IGNvbnRyb2wgKi8NCj4gKwlrdGVybWlvcyA9IHR0eS0+dGVybWlvczsN
Cj4gKwlrdGVybWlvcy5jX2NmbGFnICY9IH5DUlRTQ1RTOw0KPiArCXN0YXR1cyA9IHR0eV9zZXRf
dGVybWlvcyh0dHksICZrdGVybWlvcyk7DQo+ICsJaWYgKHN0YXR1cykNCj4gKwkJQlRfREJHKCIl
cyBkaXMgZmMgZmFpbHVyZSAlZCIsIF9fZnVuY19fLCBzdGF0dXMpOw0KPiArCWVsc2UNCj4gKwkJ
QlRfREJHKCIlcyBodyBmYyBkaXNhYmxlZCIsIF9fZnVuY19fKTsNCg0KTGV0cyBkbyBpdCBsaWtl
IHRoaXM6DQoNCglCVF9EQkco4oCcRGlzYWJsaW5nIGhhcmR3YXJlIGZsb3cgY29udHJvbDogJXPi
gJ0sIHN0YXR1cyA/IOKAnGZhaWxlZOKAnSA6IOKAnHN1Y2Nlc3PigJ0pOw0KDQo+ICsNCj4gKwkv
KiBDbGVhciBSVFMgdG8gcHJldmVudCB0aGUgZGV2aWNlIGZyb20gc2VuZGluZyAqLw0KPiArCS8q
IChtb3N0IFBDcyBuZWVkIE9VVDIgdG8gZW5hYmxlIGludGVycnVwdHMpICAgICovDQoNClBsZWFz
ZSB1c2UgdGhlIG5ldHdvcmsgc3Vic3lzdGVtIGNvbW1lbnQgc3R5bGUuDQoNCj4gKwlzdGF0dXMg
PSB0dHktPmRyaXZlci0+b3BzLT50aW9jbWdldCh0dHkpOw0KPiArCUJUX0RCRygiJXMgY3VyIHRp
b2NtIDB4JXgiLCBfX2Z1bmNfXywgc3RhdHVzKTsNCg0KCVNwZWxsIGN1cnJlbnQgb3V0IGhlcmUg
YW5kIGdldCByaWQgb2YgX19mdW5jX18NCg0KSSB3b3VsZCBhbHNvIGFkZCBhbiBleHRyYSBlbXB0
eSBsaW5lIGhlcmUgdG8gbWFrZSBpdCBhIGJpdCBtb3JlIHJlYWRhYmxlLg0KDQo+ICsJc2V0ICY9
IH4oVElPQ01fT1VUMiB8IFRJT0NNX1JUUyk7DQo+ICsJY2xlYXIgPSB+c2V0Ow0KPiArCXNldCAm
PSBUSU9DTV9EVFIgfCBUSU9DTV9SVFMgfCBUSU9DTV9PVVQxIHwNCj4gKwkJVElPQ01fT1VUMiB8
IFRJT0NNX0xPT1A7DQo+ICsJY2xlYXIgJj0gVElPQ01fRFRSIHwgVElPQ01fUlRTIHwgVElPQ01f
T1VUMSB8DQo+ICsJCVRJT0NNX09VVDIgfCBUSU9DTV9MT09QOw0KDQpZb3UgbmVlZCB0byBhbGln
biB0aGUgc2Vjb25kIGxpbmVzIGNvcnJlY3RseS4NCg0KPiArCXN0YXR1cyA9IHR0eS0+ZHJpdmVy
LT5vcHMtPnRpb2Ntc2V0KHR0eSwgc2V0LCBjbGVhcik7DQo+ICsJaWYgKHN0YXR1cykNCj4gKwkJ
QlRfREJHKCIlcyBjbHIgUlRTIGZhaWwgJWQiLCBfX2Z1bmNfXywgc3RhdHVzKTsNCj4gKwllbHNl
DQo+ICsJCUJUX0RCRygiJXMgUlRTIGNsZWFyZWQiLCBfX2Z1bmNfXyk7DQoNClNpbWlsYXIgY2hh
bmdlIGFzIGFib3ZlLg0KDQo+ICsJc3RhdHVzID0gdHR5LT5kcml2ZXItPm9wcy0+dGlvY21nZXQo
dHR5KTsNCg0KV2h5IGlzIHRoaXMgbmVlZGVkPw0KDQo+ICt9DQo+ICsNCj4gK3ZvaWQgaGNpX3Vh
cnRfdW5mbG93X2NvbnRyb2xfZGV2aWNlKHN0cnVjdCBoY2lfdWFydCAqaHUpDQo+ICt7DQo+ICsJ
c3RydWN0IHR0eV9zdHJ1Y3QgKnR0eSA9IGh1LT50dHk7DQo+ICsJc3RydWN0IGt0ZXJtaW9zIGt0
ZXJtaW9zOw0KPiArCWludCBzdGF0dXM7DQo+ICsJdW5zaWduZWQgaW50IHNldCA9IDA7DQo+ICsJ
dW5zaWduZWQgaW50IGNsZWFyID0gMDsNCj4gKw0KPiArCXN0YXR1cyA9IHR0eS0+ZHJpdmVyLT5v
cHMtPnRpb2NtZ2V0KHR0eSk7DQo+ICsJQlRfREJHKCIlcyBjdXIgdGlvY20gMHgleCIsIF9fZnVu
Y19fLCBzdGF0dXMpOw0KPiArCXNldCB8PSAoVElPQ01fT1VUMiB8IFRJT0NNX1JUUyk7DQo+ICsJ
Y2xlYXIgPSB+c2V0Ow0KPiArCXNldCAmPSBUSU9DTV9EVFIgfCBUSU9DTV9SVFMgfCBUSU9DTV9P
VVQxIHwNCj4gKwkJVElPQ01fT1VUMiB8IFRJT0NNX0xPT1A7DQo+ICsJY2xlYXIgJj0gVElPQ01f
RFRSIHwgVElPQ01fUlRTIHwgVElPQ01fT1VUMSB8DQo+ICsJCVRJT0NNX09VVDIgfCBUSU9DTV9M
T09QOw0KPiArCXN0YXR1cyA9IHR0eS0+ZHJpdmVyLT5vcHMtPnRpb2Ntc2V0KHR0eSwgc2V0LCBj
bGVhcik7DQo+ICsJaWYgKHN0YXR1cykNCj4gKwkJQlRfREJHKCIlcyBzZXQgUlRTIGZhaWwgJWQi
LCBfX2Z1bmNfXywgc3RhdHVzKTsNCj4gKwllbHNlDQo+ICsJCUJUX0RCRygiJXMgUlRTIHNldCIs
IF9fZnVuY19fKTsNCj4gKw0KPiArCS8qIFJlLWVuYWJsZSBoYXJkd2FyZSBmbG93IGNvbnRyb2wg
Ki8NCj4gKwlrdGVybWlvcyA9IHR0eS0+dGVybWlvczsNCj4gKwlrdGVybWlvcy5jX2NmbGFnIHw9
IENSVFNDVFM7DQo+ICsJc3RhdHVzID0gdHR5X3NldF90ZXJtaW9zKHR0eSwgJmt0ZXJtaW9zKTsN
Cj4gKwlpZiAoc3RhdHVzKQ0KPiArCQlCVF9EQkcoIiVzIGVuYWJsZSBmYyBmYWlsICVkIiwgX19m
dW5jX18sIHN0YXR1cyk7DQo+ICsJZWxzZQ0KPiArCQlCVF9EQkcoIiVzIGh3IGZjIHJlLWVuYWJs
ZWQiLCBfX2Z1bmNfXyk7DQo+ICt9DQoNClByZXR0eSBtdWNoIHNhbWUgbW9kaWZpY2F0aW9ucyBh
cyBmb3IgdGhlIG90aGVyIGZ1bmN0aW9uLg0KDQo+ICsNCj4gK3ZvaWQgaGNpX3VhcnRfc2V0X2Jh
dWRyYXRlKHN0cnVjdCBoY2lfdWFydCAqaHUsIHVuc2lnbmVkIGludCBzcGVlZCkNCj4gK3sNCj4g
KwlzdHJ1Y3QgdHR5X3N0cnVjdCAqdHR5ID0gaHUtPnR0eTsNCj4gKwlzdHJ1Y3Qga3Rlcm1pb3Mg
a3Rlcm1pb3M7DQo+ICsNCj4gKwkvKiBCcmluZyB0aGUgVUFSVCBpbnRvIGEga25vd24gc3RhdGUg
d2l0aCBhIGdpdmVuIGJhdWQgcmF0ZSAqLw0KPiArCWt0ZXJtaW9zID0gdHR5LT50ZXJtaW9zOw0K
PiArCWt0ZXJtaW9zLmNfY2ZsYWcgJj0gfkNCQVVEOw0KPiArCWt0ZXJtaW9zLmNfaWZsYWcgJj0g
fihJR05CUksgfCBCUktJTlQgfCBQQVJNUksgfCBJU1RSSVANCj4gKwkJfCBJTkxDUiB8IElHTkNS
IHwgSUNSTkwgfCBJWE9OKTsNCg0KRml4IHRoZSBhbGlnbm1lbnQgaGVyZS4NCg0KPiArCWt0ZXJt
aW9zLmNfb2ZsYWcgJj0gfk9QT1NUOw0KPiArCWt0ZXJtaW9zLmNfbGZsYWcgJj0gfihFQ0hPIHwg
RUNIT05MIHwgSUNBTk9OIHwgSVNJRyB8IElFWFRFTik7DQo+ICsJa3Rlcm1pb3MuY19jZmxhZyAm
PSB+KENTSVpFIHwgUEFSRU5CIHwgQ0JBVUQpOw0KPiArCWt0ZXJtaW9zLmNfY2ZsYWcgfD0gQ1M4
Ow0KPiArCWt0ZXJtaW9zLmNfY2ZsYWcgfD0gQ1JUU0NUUzsNCj4gKwkvKiBrdGVybWlvcy5jX2Nm
bGFnIHw9IEJPVEhFUjsgKi8NCg0KRG9u4oCZdCBsaWtlIHVuY29tbWVudGVkIGNvZGUuIEJldHRl
ciB0byBqdXN0IHJlbW92ZSBpdC4NCg0KPiArCXR0eV90ZXJtaW9zX2VuY29kZV9iYXVkX3JhdGUo
Jmt0ZXJtaW9zLCBzcGVlZCwgc3BlZWQpOw0KPiArDQo+ICsJLyogdHR5X3NldF90ZXJtaW9zKCkg
cmV0dXJuIG5vdCBjaGVja2VkIGFzIGl0IGlzIGFsd2F5cyAwICovDQo+ICsJdHR5X3NldF90ZXJt
aW9zKHR0eSwgJmt0ZXJtaW9zKTsNCj4gKw0KPiArCUJUX0RCRygiJXM6IE5ldyB0dHkgc3BlZWRz
OiAlZC8lZCwgY2ZsYWc6IDB4JXgiLCBodS0+aGRldi0+bmFtZSwNCj4gKwkgICAgICAgdHR5LT50
ZXJtaW9zLmNfaXNwZWVkLCB0dHktPnRlcm1pb3MuY19vc3BlZWQsDQo+ICsJICAgICAgIHR0eS0+
dGVybWlvcy5jX2NmbGFnKTsNCj4gK30NCj4gKw0KPiBzdGF0aWMgaW50IGhjaV91YXJ0X3NldHVw
KHN0cnVjdCBoY2lfZGV2ICpoZGV2KQ0KPiB7DQo+IAlzdHJ1Y3QgaGNpX3VhcnQgKmh1ID0gaGNp
X2dldF9kcnZkYXRhKGhkZXYpOw0KPiAJc3RydWN0IGhjaV9ycF9yZWFkX2xvY2FsX3ZlcnNpb24g
KnZlcjsNCj4gCXN0cnVjdCBza19idWZmICpza2I7DQo+ICsJaW50IGVycjsNCj4gKw0KPiArCWlm
IChodS0+cHJvdG8tPmluaXRfc3BlZWQpDQo+ICsJCWhjaV91YXJ0X3NldF9iYXVkcmF0ZShodSwg
aHUtPnByb3RvLT5pbml0X3NwZWVkKTsNCj4gKw0KPiArCWlmIChodS0+cHJvdG8tPnNldF9iYXVk
cmF0ZSAmJiBodS0+cHJvdG8tPm9wZXJfc3BlZWQpIHsNCj4gKwkJZXJyID0gaHUtPnByb3RvLT5z
ZXRfYmF1ZHJhdGUoaHUsIGh1LT5wcm90by0+b3Blcl9zcGVlZCk7DQo+ICsJCWlmICghZXJyKQ0K
PiArCQkJaGNpX3VhcnRfc2V0X2JhdWRyYXRlKGh1LCBodS0+cHJvdG8tPm9wZXJfc3BlZWQpOw0K
PiArCX0NCj4gDQo+IAlpZiAoaHUtPnByb3RvLT5zZXR1cCkNCj4gCQlyZXR1cm4gaHUtPnByb3Rv
LT5zZXR1cChodSk7DQo+IEBAIC02NDcsNyArNzUxLDcgQEAgc3RhdGljIGludCBfX2luaXQgaGNp
X3VhcnRfaW5pdCh2b2lkKQ0KPiANCj4gCS8qIFJlZ2lzdGVyIHRoZSB0dHkgZGlzY2lwbGluZSAq
Lw0KPiANCj4gLQltZW1zZXQoJmhjaV91YXJ0X2xkaXNjLCAwLCBzaXplb2YgKGhjaV91YXJ0X2xk
aXNjKSk7DQo+ICsJbWVtc2V0KCZoY2lfdWFydF9sZGlzYywgMCwgc2l6ZW9mKGhjaV91YXJ0X2xk
aXNjKSk7DQoNCklmIHRoaXMgaXMgd3JvbmcgaW4gZXhpc3RpbmcgY29kZSwgcGxlYXNlIHNlbmQg
YSBjbGVhbnVwIHBhdGNoIGZvciBpdC4gRWFzeSB0byBhcHBseS4NCg0KPiAJaGNpX3VhcnRfbGRp
c2MubWFnaWMJCT0gVFRZX0xESVNDX01BR0lDOw0KPiAJaGNpX3VhcnRfbGRpc2MubmFtZQkJPSAi
bl9oY2kiOw0KPiAJaGNpX3VhcnRfbGRpc2Mub3BlbgkJPSBoY2lfdWFydF90dHlfb3BlbjsNCj4g
ZGlmZiAtLWdpdCBhL2RyaXZlcnMvYmx1ZXRvb3RoL2hjaV91YXJ0LmggYi9kcml2ZXJzL2JsdWV0
b290aC9oY2lfdWFydC5oDQo+IGluZGV4IDcyMTIwYTUuLjIyNzFjYzAgMTAwNjQ0DQo+IC0tLSBh
L2RyaXZlcnMvYmx1ZXRvb3RoL2hjaV91YXJ0LmgNCj4gKysrIGIvZHJpdmVycy9ibHVldG9vdGgv
aGNpX3VhcnQuaA0KPiBAQCAtNTgsMTAgKzU4LDEzIEBAIHN0cnVjdCBoY2lfdWFydDsNCj4gc3Ry
dWN0IGhjaV91YXJ0X3Byb3RvIHsNCj4gCXVuc2lnbmVkIGludCBpZDsNCj4gCWNvbnN0IGNoYXIg
Km5hbWU7DQo+ICsJdW5zaWduZWQgaW50IGluaXRfc3BlZWQ7DQo+ICsJdW5zaWduZWQgaW50IG9w
ZXJfc3BlZWQ7DQo+IAlpbnQgKCpvcGVuKShzdHJ1Y3QgaGNpX3VhcnQgKmh1KTsNCj4gCWludCAo
KmNsb3NlKShzdHJ1Y3QgaGNpX3VhcnQgKmh1KTsNCj4gCWludCAoKmZsdXNoKShzdHJ1Y3QgaGNp
X3VhcnQgKmh1KTsNCj4gCWludCAoKnNldHVwKShzdHJ1Y3QgaGNpX3VhcnQgKmh1KTsNCj4gKwlp
bnQgKCpzZXRfYmF1ZHJhdGUpKHN0cnVjdCBoY2lfdWFydCAqaHUsIHVuc2lnbmVkIGludCBzcGVl
ZCk7DQo+IAlpbnQgKCpyZWN2KShzdHJ1Y3QgaGNpX3VhcnQgKmh1LCBjb25zdCB2b2lkICpkYXRh
LCBpbnQgbGVuKTsNCj4gCWludCAoKmVucXVldWUpKHN0cnVjdCBoY2lfdWFydCAqaHUsIHN0cnVj
dCBza19idWZmICpza2IpOw0KPiAJc3RydWN0IHNrX2J1ZmYgKigqZGVxdWV1ZSkoc3RydWN0IGhj
aV91YXJ0ICpodSk7DQo+IEBAIC05Niw2ICs5OSw5IEBAIGludCBoY2lfdWFydF9yZWdpc3Rlcl9w
cm90byhjb25zdCBzdHJ1Y3QgaGNpX3VhcnRfcHJvdG8gKnApOw0KPiBpbnQgaGNpX3VhcnRfdW5y
ZWdpc3Rlcl9wcm90byhjb25zdCBzdHJ1Y3QgaGNpX3VhcnRfcHJvdG8gKnApOw0KPiBpbnQgaGNp
X3VhcnRfdHhfd2FrZXVwKHN0cnVjdCBoY2lfdWFydCAqaHUpOw0KPiBpbnQgaGNpX3VhcnRfaW5p
dF9yZWFkeShzdHJ1Y3QgaGNpX3VhcnQgKmh1KTsNCj4gK3ZvaWQgaGNpX3VhcnRfc2V0X2JhdWRy
YXRlKHN0cnVjdCBoY2lfdWFydCAqaHUsIHVuc2lnbmVkIGludCBzcGVlZCk7DQo+ICt2b2lkIGhj
aV91YXJ0X2Zsb3dfY29udHJvbF9kZXZpY2Uoc3RydWN0IGhjaV91YXJ0ICpodSk7DQo+ICt2b2lk
IGhjaV91YXJ0X3VuZmxvd19jb250cm9sX2RldmljZShzdHJ1Y3QgaGNpX3VhcnQgKmh1KTsNCg0K
SSBhY3R1YWxseSBkaXNsaWtlIHRoaXMga2luZCBvZiBuYW1pbmcuIFdoYXQgYWJvdXQgc2ltcGxl
IHN0dWZmIGxpa2U6DQoNCgloY2lfdWFydF9lbmFibGVfZGV2aWNlKCkNCgloY2lfdWFydF9kaXNh
YmxlX2RldmljZSgpDQoNCk9yIHNvbWV0aGluZyB3aXRoIGEgYm9vbGVhbiBmb3IgdGhlIHN0YXRl
Og0KDQoJaGNpX3VhcnRfc2V0X2Zsb3dfY29udHJvbChzdHJ1Y3QgaGNpX3VhcnQgKmh1LCBib29s
IGVuYWJsZSkNCg0KUmVnYXJkcw0KDQpNYXJjZWwNCg0K

2015-06-06 11:26:24

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH 1/5] Broadcom Bluetooth UART Device Tree bindings

On 06/06/15 10:33, Marcel Holtmann wrote:
> Hi Arend,
>
>>>> Device Tree bindings to configure the Broadcom Bluetooth UART device.
>>>> Updated not to use CamelCase and to use Intel style "oper-speed".
>>>>
>>>> Signed-off-by: Ilya Faenson<[email protected]>
>>>> ---
>>>> .../devicetree/bindings/net/bluetooth/btbcm.txt | 82 ++++++++++++++++++++++
>>>> 1 file changed, 82 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
>>>> new file mode 100644
>>>> index 0000000..2679819
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
>>>> @@ -0,0 +1,82 @@
>>>> +btbcm
>>>> +------
>>>> +
>>>> +Required properties:
>>>> +
>>>> + - compatible : must be "brcm,brcm-bt-uart".
>>>> + - tty : tty device connected to this Bluetooth device.
>>>> +
>>>> +Optional properties:
>>>> +
>>>> + - bt-host-wake-gpios : bt-host-wake input GPIO to be used as an interrupt.
>>>> +
>>>> + - bt-wake-gpios : bt-wake output GPIO to be used to suspend / resume device.
>>>> +
>>>> + - bt-reg-on-gpios : reg-on output GPIO to be used to power device on/off.
>>>> +
>>>> + - oper-speed : Bluetooth device operational baud rate.
>>>> + Default: 3000000.
>>>> +
>>>> + - manual-fc : flow control UART in suspend / resume scenarios.
>>>> + Default: 0.
>>>> +
>>>> + - configure-sleep : configure suspend / resume flag.
>>>> + Default: false.
>>>> +
>>>> + - configure-audio : configure platform PCM SCO flag.
>>>> + Default: false.
>>>> +
>>>> + - pcm-clockmode : PCM clock mode. 0-slave, 1-master.
>>>> + Default: 0.
>>>> +
>>>> + - pcm-fillmethod : PCM fill method. 0 to 3.
>>>> + Default: 2.
>>>> +
>>>> + - pcm-fillnum : PCM number of fill bits. 0 to 3.
>>>> + Default: 0.
>>>> +
>>>> + - pcm-fillvalue : PCM fill value. 0 to 7.
>>>> + Default: 3.
>>>> +
>>>> + - pcm-incallbitclock : PCM interface rate. 0-128Kbps, 1-256Kbps, 2-512Kbps,
>>>> + 3-1024Kbps, 4-2048Kbps.
>>>> + Default: 0.
>>>> +
>>>> + - pcm-lsbfirst : PCM LSB first. 0 or 1.
>>>> + Default: 0.
>>>> +
>>>> + - pcm-rightjustify : PCM Justify. 0-left, 1-right.
>>>> + Default: 0.
>>>> +
>>>> + - pcm-routing : PCM routing. 0-PCM, 1-SCO over HCI.
>>>> + Default: 0.
>>>> +
>>>> + - pcm-shortframesync : PCM sync. 0-short, 1-long.
>>>> + Default: 0.
>>>> +
>>>> + - pcmsyncmode : PCM sync mode. 0-slave, 1-master.
>>>> + Default: 0.
>>>> +
>>>> +
>>>> +Example:
>>>> +
>>>> + brcm4354_bt_uart: brcm4354-bt-uart {
>>>
>>> at some point you have to explain to me if Broadcom prefers BCM or BRCM. I can not make heads or tail out of it. My current explanation is the it is BCM for the Bluetooth guys and BRCM for the WiFi guys ;)
>>
>> That's a nice explanation ;-) Maybe I can give it a shot. First, two facts: 1) BRCM is our stock symbol, and 2) our device names are prefixed by BCM. So in devicetree specifications we specified the vendor prefix as BRCM and (try to) stick to that. The above clearly refers to a device so it would probably be better to use bcm4354... there.
>>
>>>> + compatible = "brcm,brcm-bt-uart";
>>
>> The compatible string indeed starts with vendor prefix "brcm,".
>>
>>>> + bt-wake-gpios =<&gpio4 30 GPIO_ACTIVE_HIGH>;
>>>> + bt-host-wake-gpios =<&gpio4 31 GPIO_ACTIVE_HIGH>;
>>>> + tty = "ttyS0”;
>>>
>>> This tty one is the one we need to figure out. Could we reference by some node information instead of the assigned device name. If we can reference something in struct device that we know is more consistent, that would be perfect. I mean if the kernel for reason decides to enumerate things in a different order then this might be ttyS1 out of a sudden.
>>
>> I see your point, but not sure what to do here either. In devicetree you could reference the uart node, but I don't know if there is an api in the kernel to obtain tty associated with a uart port. I could not find it doing quick glance in include/linux/tty.h although the linkage is obviously there in the data types.
>
> I wonder if we get this done via ACPI in the first place. However that is something for Fred to figure out.
>
> For device tree, I bet we need to extend either the TTY subsystem or really start pushing towards the tty-slave idea that has been floating around.

The tty-slave idea popped up already in review comments. We are
wondering if it is still floating or whether things are starting to
settle in that area. Specially for the devicetree spec, which is
supposed to be a stable ABI, we probably need to take that into account
if that is indeed the way things are moving.

Ilya, this is the most recent thing I found:

http://thread.gmane.org/gmane.linux.kernel/1949724

The uart-slave idea seems to sit between tty and uart dealing with
powering the device. Not sure how that fits our need. It mentions
something about being able to intervene in any tty-uart interaction so
there could bt stuff that could be done in those hooks.

Regards,
Arend

> Regards
>
> Marcel
>

2015-06-06 08:33:18

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 1/5] Broadcom Bluetooth UART Device Tree bindings

Hi Arend,

>>> Device Tree bindings to configure the Broadcom Bluetooth UART device.
>>> Updated not to use CamelCase and to use Intel style "oper-speed".
>>>
>>> Signed-off-by: Ilya Faenson<[email protected]>
>>> ---
>>> .../devicetree/bindings/net/bluetooth/btbcm.txt | 82 ++++++++++++++++++++++
>>> 1 file changed, 82 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
>>> new file mode 100644
>>> index 0000000..2679819
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
>>> @@ -0,0 +1,82 @@
>>> +btbcm
>>> +------
>>> +
>>> +Required properties:
>>> +
>>> + - compatible : must be "brcm,brcm-bt-uart".
>>> + - tty : tty device connected to this Bluetooth device.
>>> +
>>> +Optional properties:
>>> +
>>> + - bt-host-wake-gpios : bt-host-wake input GPIO to be used as an interrupt.
>>> +
>>> + - bt-wake-gpios : bt-wake output GPIO to be used to suspend / resume device.
>>> +
>>> + - bt-reg-on-gpios : reg-on output GPIO to be used to power device on/off.
>>> +
>>> + - oper-speed : Bluetooth device operational baud rate.
>>> + Default: 3000000.
>>> +
>>> + - manual-fc : flow control UART in suspend / resume scenarios.
>>> + Default: 0.
>>> +
>>> + - configure-sleep : configure suspend / resume flag.
>>> + Default: false.
>>> +
>>> + - configure-audio : configure platform PCM SCO flag.
>>> + Default: false.
>>> +
>>> + - pcm-clockmode : PCM clock mode. 0-slave, 1-master.
>>> + Default: 0.
>>> +
>>> + - pcm-fillmethod : PCM fill method. 0 to 3.
>>> + Default: 2.
>>> +
>>> + - pcm-fillnum : PCM number of fill bits. 0 to 3.
>>> + Default: 0.
>>> +
>>> + - pcm-fillvalue : PCM fill value. 0 to 7.
>>> + Default: 3.
>>> +
>>> + - pcm-incallbitclock : PCM interface rate. 0-128Kbps, 1-256Kbps, 2-512Kbps,
>>> + 3-1024Kbps, 4-2048Kbps.
>>> + Default: 0.
>>> +
>>> + - pcm-lsbfirst : PCM LSB first. 0 or 1.
>>> + Default: 0.
>>> +
>>> + - pcm-rightjustify : PCM Justify. 0-left, 1-right.
>>> + Default: 0.
>>> +
>>> + - pcm-routing : PCM routing. 0-PCM, 1-SCO over HCI.
>>> + Default: 0.
>>> +
>>> + - pcm-shortframesync : PCM sync. 0-short, 1-long.
>>> + Default: 0.
>>> +
>>> + - pcmsyncmode : PCM sync mode. 0-slave, 1-master.
>>> + Default: 0.
>>> +
>>> +
>>> +Example:
>>> +
>>> + brcm4354_bt_uart: brcm4354-bt-uart {
>>
>> at some point you have to explain to me if Broadcom prefers BCM or BRCM. I can not make heads or tail out of it. My current explanation is the it is BCM for the Bluetooth guys and BRCM for the WiFi guys ;)
>
> That's a nice explanation ;-) Maybe I can give it a shot. First, two facts: 1) BRCM is our stock symbol, and 2) our device names are prefixed by BCM. So in devicetree specifications we specified the vendor prefix as BRCM and (try to) stick to that. The above clearly refers to a device so it would probably be better to use bcm4354... there.
>
>>> + compatible = "brcm,brcm-bt-uart";
>
> The compatible string indeed starts with vendor prefix "brcm,".
>
>>> + bt-wake-gpios =<&gpio4 30 GPIO_ACTIVE_HIGH>;
>>> + bt-host-wake-gpios =<&gpio4 31 GPIO_ACTIVE_HIGH>;
>>> + tty = "ttyS0”;
>>
>> This tty one is the one we need to figure out. Could we reference by some node information instead of the assigned device name. If we can reference something in struct device that we know is more consistent, that would be perfect. I mean if the kernel for reason decides to enumerate things in a different order then this might be ttyS1 out of a sudden.
>
> I see your point, but not sure what to do here either. In devicetree you could reference the uart node, but I don't know if there is an api in the kernel to obtain tty associated with a uart port. I could not find it doing quick glance in include/linux/tty.h although the linkage is obviously there in the data types.

I wonder if we get this done via ACPI in the first place. However that is something for Fred to figure out.

For device tree, I bet we need to extend either the TTY subsystem or really start pushing towards the tty-slave idea that has been floating around.

Regards

Marcel


2015-06-06 08:31:44

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 4/5] Broadcom Bluetooth protocol UART support

Hi Arend,

>>> Merge of the Broadcom logic into the Intel's btbcm implementation.
>>>
>>> Signed-off-by: Ilya Faenson<[email protected]>
>>> ---
>>> drivers/bluetooth/btbcm.c | 142 ++++++++++++++++++++++++++++++++++++----------
>>> drivers/bluetooth/btbcm.h | 21 ++++++-
>>> 2 files changed, 132 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
>>> index 728fce3..e1d5ad0 100644
>>> --- a/drivers/bluetooth/btbcm.c
>>> +++ b/drivers/bluetooth/btbcm.c
>>> @@ -3,6 +3,7 @@
>>> * Bluetooth support for Broadcom devices
>>> *
>>> * Copyright (C) 2015 Intel Corporation
>>> + * Copyright (C) 2015 Broadcom Corporation
>>> *
>>> *
>>> * This program is free software; you can redistribute it and/or modify
>>> @@ -32,7 +33,8 @@
>>>
>>> #define VERSION "0.1"
>>>
>>> -#define BDADDR_BCM20702A0 (&(bdaddr_t) {{0x00, 0xa0, 0x02, 0x70, 0x20, 0x00}})
>>> +#define BDADDR_BCM20702A0 (&(bdaddr_t) {{0x00, 0xa0, 0x02, 0x70, 0x20, 0x00} })
>>> +#define BDADDR_BCM4324B3 (&(bdaddr_t) {{0x00, 0x00, 0x00, 0xb3, 0x24, 0x43} })
>>
>> something funky is going on with your editor that insist on fixing this ;)
>>
>>>
>>> int btbcm_check_bdaddr(struct hci_dev *hdev)
>>> {
>>> @@ -43,6 +45,7 @@ int btbcm_check_bdaddr(struct hci_dev *hdev)
>>> HCI_INIT_TIMEOUT);
>>> if (IS_ERR(skb)) {
>>> int err = PTR_ERR(skb);
>>> +
>>
>> In this specific case, please do not fix it. I like to keep the simple error path sections small.
>>
>>> BT_ERR("%s: BCM: Reading device address failed (%d)",
>>> hdev->name, err);
>>> return err;
>>> @@ -56,10 +59,11 @@ int btbcm_check_bdaddr(struct hci_dev *hdev)
>>>
>>> bda = (struct hci_rp_read_bd_addr *)skb->data;
>>>
>>> - /* The address 00:20:70:02:A0:00 indicates a BCM20702A0 controller
>>> - * with no configured address.
>>> + /* Check if the address indicates a controller with no configured
>>> + * address.
>>> */
>>> - if (!bacmp(&bda->bdaddr, BDADDR_BCM20702A0)) {
>>> + if (!bacmp(&bda->bdaddr, BDADDR_BCM20702A0) ||
>>> + !bacmp(&bda->bdaddr, BDADDR_BCM4324B3)) {
>>> BT_INFO("%s: BCM: Using default device address (%pMR)",
>>> hdev->name,&bda->bdaddr);
>>> set_bit(HCI_QUIRK_INVALID_BDADDR,&hdev->quirks);
>>> @@ -89,21 +93,14 @@ int btbcm_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
>>> }
>>> EXPORT_SYMBOL_GPL(btbcm_set_bdaddr);
>>>
>>> -int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
>>> +int btbcm_patchram(struct hci_dev *hdev, const struct firmware *fw)
>>> {
>>> const struct hci_command_hdr *cmd;
>>> - const struct firmware *fw;
>>> const u8 *fw_ptr;
>>> size_t fw_size;
>>> struct sk_buff *skb;
>>> u16 opcode;
>>> - int err;
>>> -
>>> - err = request_firmware(&fw, firmware,&hdev->dev);
>>> - if (err< 0) {
>>> - BT_INFO("%s: BCM: Patch %s not found", hdev->name, firmware);
>>> - return err;
>>> - }
>>> + int err = 0;
>>>
>>> /* Start Download */
>>> skb = __hci_cmd_sync(hdev, 0xfc2e, 0, NULL, HCI_INIT_TIMEOUT);
>>> @@ -129,8 +126,7 @@ int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
>>> fw_size -= sizeof(*cmd);
>>>
>>> if (fw_size< cmd->plen) {
>>> - BT_ERR("%s: BCM: Patch %s is corrupted", hdev->name,
>>> - firmware);
>>> + BT_ERR("%s: BCM: Patch is corrupted", hdev->name);
>>> err = -EINVAL;
>>> goto done;
>>> }
>>> @@ -156,7 +152,6 @@ int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
>>> msleep(250);
>>>
>>> done:
>>> - release_firmware(fw);
>>> return err;
>>> }
>>> EXPORT_SYMBOL(btbcm_patchram);
>>> @@ -168,6 +163,7 @@ static int btbcm_reset(struct hci_dev *hdev)
>>> skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
>>> if (IS_ERR(skb)) {
>>> int err = PTR_ERR(skb);
>>> +
>>> BT_ERR("%s: BCM: Reset failed (%d)", hdev->name, err);
>>> return err;
>>> }
>>> @@ -242,9 +238,101 @@ static const struct {
>>> const char *name;
>>> } bcm_uart_subver_table[] = {
>>> { 0x410e, "BCM43341B0" }, /* 002.001.014 */
>>> + { 0x4406, "BCM4324B3" }, /* 002.004.006 */
>>> + { 0x610c, "BCM4354_003.001.012.0306.0659"}, /* 003.001.012 */
>>
>> Please just BCM4354 here.
>>
>> And I have initial patches for the manifest file to map modalias to firmware names. That will then hopefully solve all of these and we can just update this easily from userspace. I will send patches as soon as I have cleaned them up a bit.
>
> Is using modalias sufficient. At least for our wifi chips it is not as new chip revisions with same modalias require different firmware. Same may be true for bt chips.

let me put it this way, the Windows driver maps the firmware based on USB VID/PID and thus that seems fine then. Also it seems that every ACPI based platform gets a proper ID assigned. So that makes sense as well there.

So there are plenty of USB dongles that share the same firmware, but it seems to be manual mapping based on who tested and certified a given firmware for a given piece of hardware. It is just too many that I really want to know. I rather have them all listed and then someone can update it from userspace instead of having to deal with it in the kernel.

Regards

Marcel


2015-06-06 08:15:33

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH 4/5] Broadcom Bluetooth protocol UART support

On 06/06/15 08:37, Marcel Holtmann wrote:
> Hi Ilya,
>
>> Merge of the Broadcom logic into the Intel's btbcm implementation.
>>
>> Signed-off-by: Ilya Faenson<[email protected]>
>> ---
>> drivers/bluetooth/btbcm.c | 142 ++++++++++++++++++++++++++++++++++++----------
>> drivers/bluetooth/btbcm.h | 21 ++++++-
>> 2 files changed, 132 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
>> index 728fce3..e1d5ad0 100644
>> --- a/drivers/bluetooth/btbcm.c
>> +++ b/drivers/bluetooth/btbcm.c
>> @@ -3,6 +3,7 @@
>> * Bluetooth support for Broadcom devices
>> *
>> * Copyright (C) 2015 Intel Corporation
>> + * Copyright (C) 2015 Broadcom Corporation
>> *
>> *
>> * This program is free software; you can redistribute it and/or modify
>> @@ -32,7 +33,8 @@
>>
>> #define VERSION "0.1"
>>
>> -#define BDADDR_BCM20702A0 (&(bdaddr_t) {{0x00, 0xa0, 0x02, 0x70, 0x20, 0x00}})
>> +#define BDADDR_BCM20702A0 (&(bdaddr_t) {{0x00, 0xa0, 0x02, 0x70, 0x20, 0x00} })
>> +#define BDADDR_BCM4324B3 (&(bdaddr_t) {{0x00, 0x00, 0x00, 0xb3, 0x24, 0x43} })
>
> something funky is going on with your editor that insist on fixing this ;)
>
>>
>> int btbcm_check_bdaddr(struct hci_dev *hdev)
>> {
>> @@ -43,6 +45,7 @@ int btbcm_check_bdaddr(struct hci_dev *hdev)
>> HCI_INIT_TIMEOUT);
>> if (IS_ERR(skb)) {
>> int err = PTR_ERR(skb);
>> +
>
> In this specific case, please do not fix it. I like to keep the simple error path sections small.
>
>> BT_ERR("%s: BCM: Reading device address failed (%d)",
>> hdev->name, err);
>> return err;
>> @@ -56,10 +59,11 @@ int btbcm_check_bdaddr(struct hci_dev *hdev)
>>
>> bda = (struct hci_rp_read_bd_addr *)skb->data;
>>
>> - /* The address 00:20:70:02:A0:00 indicates a BCM20702A0 controller
>> - * with no configured address.
>> + /* Check if the address indicates a controller with no configured
>> + * address.
>> */
>> - if (!bacmp(&bda->bdaddr, BDADDR_BCM20702A0)) {
>> + if (!bacmp(&bda->bdaddr, BDADDR_BCM20702A0) ||
>> + !bacmp(&bda->bdaddr, BDADDR_BCM4324B3)) {
>> BT_INFO("%s: BCM: Using default device address (%pMR)",
>> hdev->name,&bda->bdaddr);
>> set_bit(HCI_QUIRK_INVALID_BDADDR,&hdev->quirks);
>> @@ -89,21 +93,14 @@ int btbcm_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
>> }
>> EXPORT_SYMBOL_GPL(btbcm_set_bdaddr);
>>
>> -int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
>> +int btbcm_patchram(struct hci_dev *hdev, const struct firmware *fw)
>> {
>> const struct hci_command_hdr *cmd;
>> - const struct firmware *fw;
>> const u8 *fw_ptr;
>> size_t fw_size;
>> struct sk_buff *skb;
>> u16 opcode;
>> - int err;
>> -
>> - err = request_firmware(&fw, firmware,&hdev->dev);
>> - if (err< 0) {
>> - BT_INFO("%s: BCM: Patch %s not found", hdev->name, firmware);
>> - return err;
>> - }
>> + int err = 0;
>>
>> /* Start Download */
>> skb = __hci_cmd_sync(hdev, 0xfc2e, 0, NULL, HCI_INIT_TIMEOUT);
>> @@ -129,8 +126,7 @@ int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
>> fw_size -= sizeof(*cmd);
>>
>> if (fw_size< cmd->plen) {
>> - BT_ERR("%s: BCM: Patch %s is corrupted", hdev->name,
>> - firmware);
>> + BT_ERR("%s: BCM: Patch is corrupted", hdev->name);
>> err = -EINVAL;
>> goto done;
>> }
>> @@ -156,7 +152,6 @@ int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
>> msleep(250);
>>
>> done:
>> - release_firmware(fw);
>> return err;
>> }
>> EXPORT_SYMBOL(btbcm_patchram);
>> @@ -168,6 +163,7 @@ static int btbcm_reset(struct hci_dev *hdev)
>> skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
>> if (IS_ERR(skb)) {
>> int err = PTR_ERR(skb);
>> +
>> BT_ERR("%s: BCM: Reset failed (%d)", hdev->name, err);
>> return err;
>> }
>> @@ -242,9 +238,101 @@ static const struct {
>> const char *name;
>> } bcm_uart_subver_table[] = {
>> { 0x410e, "BCM43341B0" }, /* 002.001.014 */
>> + { 0x4406, "BCM4324B3" }, /* 002.004.006 */
>> + { 0x610c, "BCM4354_003.001.012.0306.0659"}, /* 003.001.012 */
>
> Please just BCM4354 here.
>
> And I have initial patches for the manifest file to map modalias to firmware names. That will then hopefully solve all of these and we can just update this easily from userspace. I will send patches as soon as I have cleaned them up a bit.

Hi Marcel,

Is using modalias sufficient. At least for our wifi chips it is not as
new chip revisions with same modalias require different firmware. Same
may be true for bt chips.

Regards,
Arend

> Regards
>
> Marcel
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2015-06-06 07:41:03

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH 1/5] Broadcom Bluetooth UART Device Tree bindings

On 06/06/15 08:40, Marcel Holtmann wrote:
> Hi Ilya,
>
>> Device Tree bindings to configure the Broadcom Bluetooth UART device.
>> Updated not to use CamelCase and to use Intel style "oper-speed".
>>
>> Signed-off-by: Ilya Faenson<[email protected]>
>> ---
>> .../devicetree/bindings/net/bluetooth/btbcm.txt | 82 ++++++++++++++++++++++
>> 1 file changed, 82 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
>> new file mode 100644
>> index 0000000..2679819
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
>> @@ -0,0 +1,82 @@
>> +btbcm
>> +------
>> +
>> +Required properties:
>> +
>> + - compatible : must be "brcm,brcm-bt-uart".
>> + - tty : tty device connected to this Bluetooth device.
>> +
>> +Optional properties:
>> +
>> + - bt-host-wake-gpios : bt-host-wake input GPIO to be used as an interrupt.
>> +
>> + - bt-wake-gpios : bt-wake output GPIO to be used to suspend / resume device.
>> +
>> + - bt-reg-on-gpios : reg-on output GPIO to be used to power device on/off.
>> +
>> + - oper-speed : Bluetooth device operational baud rate.
>> + Default: 3000000.
>> +
>> + - manual-fc : flow control UART in suspend / resume scenarios.
>> + Default: 0.
>> +
>> + - configure-sleep : configure suspend / resume flag.
>> + Default: false.
>> +
>> + - configure-audio : configure platform PCM SCO flag.
>> + Default: false.
>> +
>> + - pcm-clockmode : PCM clock mode. 0-slave, 1-master.
>> + Default: 0.
>> +
>> + - pcm-fillmethod : PCM fill method. 0 to 3.
>> + Default: 2.
>> +
>> + - pcm-fillnum : PCM number of fill bits. 0 to 3.
>> + Default: 0.
>> +
>> + - pcm-fillvalue : PCM fill value. 0 to 7.
>> + Default: 3.
>> +
>> + - pcm-incallbitclock : PCM interface rate. 0-128Kbps, 1-256Kbps, 2-512Kbps,
>> + 3-1024Kbps, 4-2048Kbps.
>> + Default: 0.
>> +
>> + - pcm-lsbfirst : PCM LSB first. 0 or 1.
>> + Default: 0.
>> +
>> + - pcm-rightjustify : PCM Justify. 0-left, 1-right.
>> + Default: 0.
>> +
>> + - pcm-routing : PCM routing. 0-PCM, 1-SCO over HCI.
>> + Default: 0.
>> +
>> + - pcm-shortframesync : PCM sync. 0-short, 1-long.
>> + Default: 0.
>> +
>> + - pcmsyncmode : PCM sync mode. 0-slave, 1-master.
>> + Default: 0.
>> +
>> +
>> +Example:
>> +
>> + brcm4354_bt_uart: brcm4354-bt-uart {
>
> at some point you have to explain to me if Broadcom prefers BCM or BRCM. I can not make heads or tail out of it. My current explanation is the it is BCM for the Bluetooth guys and BRCM for the WiFi guys ;)

That's a nice explanation ;-) Maybe I can give it a shot. First, two
facts: 1) BRCM is our stock symbol, and 2) our device names are prefixed
by BCM. So in devicetree specifications we specified the vendor prefix
as BRCM and (try to) stick to that. The above clearly refers to a device
so it would probably be better to use bcm4354... there.

>> + compatible = "brcm,brcm-bt-uart";

The compatible string indeed starts with vendor prefix "brcm,".

>> + bt-wake-gpios =<&gpio4 30 GPIO_ACTIVE_HIGH>;
>> + bt-host-wake-gpios =<&gpio4 31 GPIO_ACTIVE_HIGH>;
>> + tty = "ttyS0”;
>
> This tty one is the one we need to figure out. Could we reference by some node information instead of the assigned device name. If we can reference something in struct device that we know is more consistent, that would be perfect. I mean if the kernel for reason decides to enumerate things in a different order then this might be ttyS1 out of a sudden.

I see your point, but not sure what to do here either. In devicetree you
could reference the uart node, but I don't know if there is an api in
the kernel to obtain tty associated with a uart port. I could not find
it doing quick glance in include/linux/tty.h although the linkage is
obviously there in the data types.

Regards,
Arend

2015-06-06 06:40:34

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 1/5] Broadcom Bluetooth UART Device Tree bindings

Hi Ilya,

> Device Tree bindings to configure the Broadcom Bluetooth UART device.
> Updated not to use CamelCase and to use Intel style "oper-speed".
>
> Signed-off-by: Ilya Faenson <[email protected]>
> ---
> .../devicetree/bindings/net/bluetooth/btbcm.txt | 82 ++++++++++++++++++++++
> 1 file changed, 82 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
>
> diff --git a/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
> new file mode 100644
> index 0000000..2679819
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
> @@ -0,0 +1,82 @@
> +btbcm
> +------
> +
> +Required properties:
> +
> + - compatible : must be "brcm,brcm-bt-uart".
> + - tty : tty device connected to this Bluetooth device.
> +
> +Optional properties:
> +
> + - bt-host-wake-gpios : bt-host-wake input GPIO to be used as an interrupt.
> +
> + - bt-wake-gpios : bt-wake output GPIO to be used to suspend / resume device.
> +
> + - bt-reg-on-gpios : reg-on output GPIO to be used to power device on/off.
> +
> + - oper-speed : Bluetooth device operational baud rate.
> + Default: 3000000.
> +
> + - manual-fc : flow control UART in suspend / resume scenarios.
> + Default: 0.
> +
> + - configure-sleep : configure suspend / resume flag.
> + Default: false.
> +
> + - configure-audio : configure platform PCM SCO flag.
> + Default: false.
> +
> + - pcm-clockmode : PCM clock mode. 0-slave, 1-master.
> + Default: 0.
> +
> + - pcm-fillmethod : PCM fill method. 0 to 3.
> + Default: 2.
> +
> + - pcm-fillnum : PCM number of fill bits. 0 to 3.
> + Default: 0.
> +
> + - pcm-fillvalue : PCM fill value. 0 to 7.
> + Default: 3.
> +
> + - pcm-incallbitclock : PCM interface rate. 0-128Kbps, 1-256Kbps, 2-512Kbps,
> + 3-1024Kbps, 4-2048Kbps.
> + Default: 0.
> +
> + - pcm-lsbfirst : PCM LSB first. 0 or 1.
> + Default: 0.
> +
> + - pcm-rightjustify : PCM Justify. 0-left, 1-right.
> + Default: 0.
> +
> + - pcm-routing : PCM routing. 0-PCM, 1-SCO over HCI.
> + Default: 0.
> +
> + - pcm-shortframesync : PCM sync. 0-short, 1-long.
> + Default: 0.
> +
> + - pcmsyncmode : PCM sync mode. 0-slave, 1-master.
> + Default: 0.
> +
> +
> +Example:
> +
> + brcm4354_bt_uart: brcm4354-bt-uart {

at some point you have to explain to me if Broadcom prefers BCM or BRCM. I can not make heads or tail out of it. My current explanation is the it is BCM for the Bluetooth guys and BRCM for the WiFi guys ;)

> + compatible = "brcm,brcm-bt-uart";
> + bt-wake-gpios = <&gpio4 30 GPIO_ACTIVE_HIGH>;
> + bt-host-wake-gpios = <&gpio4 31 GPIO_ACTIVE_HIGH>;
> + tty = "ttyS0”;

This tty one is the one we need to figure out. Could we reference by some node information instead of the assigned device name. If we can reference something in struct device that we know is more consistent, that would be perfect. I mean if the kernel for reason decides to enumerate things in a different order then this might be ttyS1 out of a sudden.

Regards

Marcel


2015-06-06 06:36:58

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 2/5] Intel based H4 line discipline enhancements

Hi Ilya,

> This is largely enhancements implemented by Frederic Danis of Intel.
> I've also added the ability to flow control the UART and improved the
> UART baud rate setting some.

I applied parts of Fred’s patches and so please rebase on top of them. Makes it a lot easier for me to review changes instead of figuring out what comes from you and what comes from Fred.

>
> Signed-off-by: Ilya Faenson <[email protected]>
> ---
> drivers/bluetooth/hci_ldisc.c | 110 ++++++++++++++++++++++++++++++++++++++++--
> drivers/bluetooth/hci_uart.h | 6 +++
> 2 files changed, 113 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
> index 5c9a73f..58dcb24 100644
> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -1,4 +1,4 @@
> -/*
> +/*_

Funky change ;)

> *
> * Bluetooth HCI UART driver
> *
> @@ -256,7 +256,8 @@ static int hci_uart_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
> if (!test_bit(HCI_RUNNING, &hdev->flags))
> return -EBUSY;
>
> - BT_DBG("%s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type, skb->len);
> + BT_DBG("%s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type,
> + skb->len);

In case you spot coding style errors, please just send them as tiny cleanup patches. I can easily apply them out of order and we get of this. Makes review a lot easier.

>
> hu->proto->enqueue(hu, skb);
>
> @@ -265,11 +266,114 @@ static int hci_uart_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
> return 0;
> }
>
> +void hci_uart_flow_control_device(struct hci_uart *hu)
> +{
> + struct tty_struct *tty = hu->tty;
> + struct ktermios ktermios;
> + int status;
> + unsigned int set = 0;
> + unsigned int clear = 0;
> +
> + /* Disable hardware flow control */
> + ktermios = tty->termios;
> + ktermios.c_cflag &= ~CRTSCTS;
> + status = tty_set_termios(tty, &ktermios);
> + if (status)
> + BT_DBG("%s dis fc failure %d", __func__, status);
> + else
> + BT_DBG("%s hw fc disabled", __func__);

Lets do it like this:

BT_DBG(“Disabling hardware flow control: %s”, status ? “failed” : “success”);

> +
> + /* Clear RTS to prevent the device from sending */
> + /* (most PCs need OUT2 to enable interrupts) */

Please use the network subsystem comment style.

> + status = tty->driver->ops->tiocmget(tty);
> + BT_DBG("%s cur tiocm 0x%x", __func__, status);

Spell current out here and get rid of __func__

I would also add an extra empty line here to make it a bit more readable.

> + set &= ~(TIOCM_OUT2 | TIOCM_RTS);
> + clear = ~set;
> + set &= TIOCM_DTR | TIOCM_RTS | TIOCM_OUT1 |
> + TIOCM_OUT2 | TIOCM_LOOP;
> + clear &= TIOCM_DTR | TIOCM_RTS | TIOCM_OUT1 |
> + TIOCM_OUT2 | TIOCM_LOOP;

You need to align the second lines correctly.

> + status = tty->driver->ops->tiocmset(tty, set, clear);
> + if (status)
> + BT_DBG("%s clr RTS fail %d", __func__, status);
> + else
> + BT_DBG("%s RTS cleared", __func__);

Similar change as above.

> + status = tty->driver->ops->tiocmget(tty);

Why is this needed?

> +}
> +
> +void hci_uart_unflow_control_device(struct hci_uart *hu)
> +{
> + struct tty_struct *tty = hu->tty;
> + struct ktermios ktermios;
> + int status;
> + unsigned int set = 0;
> + unsigned int clear = 0;
> +
> + status = tty->driver->ops->tiocmget(tty);
> + BT_DBG("%s cur tiocm 0x%x", __func__, status);
> + set |= (TIOCM_OUT2 | TIOCM_RTS);
> + clear = ~set;
> + set &= TIOCM_DTR | TIOCM_RTS | TIOCM_OUT1 |
> + TIOCM_OUT2 | TIOCM_LOOP;
> + clear &= TIOCM_DTR | TIOCM_RTS | TIOCM_OUT1 |
> + TIOCM_OUT2 | TIOCM_LOOP;
> + status = tty->driver->ops->tiocmset(tty, set, clear);
> + if (status)
> + BT_DBG("%s set RTS fail %d", __func__, status);
> + else
> + BT_DBG("%s RTS set", __func__);
> +
> + /* Re-enable hardware flow control */
> + ktermios = tty->termios;
> + ktermios.c_cflag |= CRTSCTS;
> + status = tty_set_termios(tty, &ktermios);
> + if (status)
> + BT_DBG("%s enable fc fail %d", __func__, status);
> + else
> + BT_DBG("%s hw fc re-enabled", __func__);
> +}

Pretty much same modifications as for the other function.

> +
> +void hci_uart_set_baudrate(struct hci_uart *hu, unsigned int speed)
> +{
> + struct tty_struct *tty = hu->tty;
> + struct ktermios ktermios;
> +
> + /* Bring the UART into a known state with a given baud rate */
> + ktermios = tty->termios;
> + ktermios.c_cflag &= ~CBAUD;
> + ktermios.c_iflag &= ~(IGNBRK | BRKINT | PARMRK | ISTRIP
> + | INLCR | IGNCR | ICRNL | IXON);

Fix the alignment here.

> + ktermios.c_oflag &= ~OPOST;
> + ktermios.c_lflag &= ~(ECHO | ECHONL | ICANON | ISIG | IEXTEN);
> + ktermios.c_cflag &= ~(CSIZE | PARENB | CBAUD);
> + ktermios.c_cflag |= CS8;
> + ktermios.c_cflag |= CRTSCTS;
> + /* ktermios.c_cflag |= BOTHER; */

Don’t like uncommented code. Better to just remove it.

> + tty_termios_encode_baud_rate(&ktermios, speed, speed);
> +
> + /* tty_set_termios() return not checked as it is always 0 */
> + tty_set_termios(tty, &ktermios);
> +
> + BT_DBG("%s: New tty speeds: %d/%d, cflag: 0x%x", hu->hdev->name,
> + tty->termios.c_ispeed, tty->termios.c_ospeed,
> + tty->termios.c_cflag);
> +}
> +
> static int hci_uart_setup(struct hci_dev *hdev)
> {
> struct hci_uart *hu = hci_get_drvdata(hdev);
> struct hci_rp_read_local_version *ver;
> struct sk_buff *skb;
> + int err;
> +
> + if (hu->proto->init_speed)
> + hci_uart_set_baudrate(hu, hu->proto->init_speed);
> +
> + if (hu->proto->set_baudrate && hu->proto->oper_speed) {
> + err = hu->proto->set_baudrate(hu, hu->proto->oper_speed);
> + if (!err)
> + hci_uart_set_baudrate(hu, hu->proto->oper_speed);
> + }
>
> if (hu->proto->setup)
> return hu->proto->setup(hu);
> @@ -647,7 +751,7 @@ static int __init hci_uart_init(void)
>
> /* Register the tty discipline */
>
> - memset(&hci_uart_ldisc, 0, sizeof (hci_uart_ldisc));
> + memset(&hci_uart_ldisc, 0, sizeof(hci_uart_ldisc));

If this is wrong in existing code, please send a cleanup patch for it. Easy to apply.

> hci_uart_ldisc.magic = TTY_LDISC_MAGIC;
> hci_uart_ldisc.name = "n_hci";
> hci_uart_ldisc.open = hci_uart_tty_open;
> diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
> index 72120a5..2271cc0 100644
> --- a/drivers/bluetooth/hci_uart.h
> +++ b/drivers/bluetooth/hci_uart.h
> @@ -58,10 +58,13 @@ struct hci_uart;
> struct hci_uart_proto {
> unsigned int id;
> const char *name;
> + unsigned int init_speed;
> + unsigned int oper_speed;
> int (*open)(struct hci_uart *hu);
> int (*close)(struct hci_uart *hu);
> int (*flush)(struct hci_uart *hu);
> int (*setup)(struct hci_uart *hu);
> + int (*set_baudrate)(struct hci_uart *hu, unsigned int speed);
> int (*recv)(struct hci_uart *hu, const void *data, int len);
> int (*enqueue)(struct hci_uart *hu, struct sk_buff *skb);
> struct sk_buff *(*dequeue)(struct hci_uart *hu);
> @@ -96,6 +99,9 @@ int hci_uart_register_proto(const struct hci_uart_proto *p);
> int hci_uart_unregister_proto(const struct hci_uart_proto *p);
> int hci_uart_tx_wakeup(struct hci_uart *hu);
> int hci_uart_init_ready(struct hci_uart *hu);
> +void hci_uart_set_baudrate(struct hci_uart *hu, unsigned int speed);
> +void hci_uart_flow_control_device(struct hci_uart *hu);
> +void hci_uart_unflow_control_device(struct hci_uart *hu);

I actually dislike this kind of naming. What about simple stuff like:

hci_uart_enable_device()
hci_uart_disable_device()

Or something with a boolean for the state:

hci_uart_set_flow_control(struct hci_uart *hu, bool enable)

Regards

Marcel


2015-06-06 06:37:01

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 4/5] Broadcom Bluetooth protocol UART support

Hi Ilya,

> Merge of the Broadcom logic into the Intel's btbcm implementation.
>
> Signed-off-by: Ilya Faenson <[email protected]>
> ---
> drivers/bluetooth/btbcm.c | 142 ++++++++++++++++++++++++++++++++++++----------
> drivers/bluetooth/btbcm.h | 21 ++++++-
> 2 files changed, 132 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
> index 728fce3..e1d5ad0 100644
> --- a/drivers/bluetooth/btbcm.c
> +++ b/drivers/bluetooth/btbcm.c
> @@ -3,6 +3,7 @@
> * Bluetooth support for Broadcom devices
> *
> * Copyright (C) 2015 Intel Corporation
> + * Copyright (C) 2015 Broadcom Corporation
> *
> *
> * This program is free software; you can redistribute it and/or modify
> @@ -32,7 +33,8 @@
>
> #define VERSION "0.1"
>
> -#define BDADDR_BCM20702A0 (&(bdaddr_t) {{0x00, 0xa0, 0x02, 0x70, 0x20, 0x00}})
> +#define BDADDR_BCM20702A0 (&(bdaddr_t) {{0x00, 0xa0, 0x02, 0x70, 0x20, 0x00} })
> +#define BDADDR_BCM4324B3 (&(bdaddr_t) {{0x00, 0x00, 0x00, 0xb3, 0x24, 0x43} })

something funky is going on with your editor that insist on fixing this ;)

>
> int btbcm_check_bdaddr(struct hci_dev *hdev)
> {
> @@ -43,6 +45,7 @@ int btbcm_check_bdaddr(struct hci_dev *hdev)
> HCI_INIT_TIMEOUT);
> if (IS_ERR(skb)) {
> int err = PTR_ERR(skb);
> +

In this specific case, please do not fix it. I like to keep the simple error path sections small.

> BT_ERR("%s: BCM: Reading device address failed (%d)",
> hdev->name, err);
> return err;
> @@ -56,10 +59,11 @@ int btbcm_check_bdaddr(struct hci_dev *hdev)
>
> bda = (struct hci_rp_read_bd_addr *)skb->data;
>
> - /* The address 00:20:70:02:A0:00 indicates a BCM20702A0 controller
> - * with no configured address.
> + /* Check if the address indicates a controller with no configured
> + * address.
> */
> - if (!bacmp(&bda->bdaddr, BDADDR_BCM20702A0)) {
> + if (!bacmp(&bda->bdaddr, BDADDR_BCM20702A0) ||
> + !bacmp(&bda->bdaddr, BDADDR_BCM4324B3)) {
> BT_INFO("%s: BCM: Using default device address (%pMR)",
> hdev->name, &bda->bdaddr);
> set_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks);
> @@ -89,21 +93,14 @@ int btbcm_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
> }
> EXPORT_SYMBOL_GPL(btbcm_set_bdaddr);
>
> -int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
> +int btbcm_patchram(struct hci_dev *hdev, const struct firmware *fw)
> {
> const struct hci_command_hdr *cmd;
> - const struct firmware *fw;
> const u8 *fw_ptr;
> size_t fw_size;
> struct sk_buff *skb;
> u16 opcode;
> - int err;
> -
> - err = request_firmware(&fw, firmware, &hdev->dev);
> - if (err < 0) {
> - BT_INFO("%s: BCM: Patch %s not found", hdev->name, firmware);
> - return err;
> - }
> + int err = 0;
>
> /* Start Download */
> skb = __hci_cmd_sync(hdev, 0xfc2e, 0, NULL, HCI_INIT_TIMEOUT);
> @@ -129,8 +126,7 @@ int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
> fw_size -= sizeof(*cmd);
>
> if (fw_size < cmd->plen) {
> - BT_ERR("%s: BCM: Patch %s is corrupted", hdev->name,
> - firmware);
> + BT_ERR("%s: BCM: Patch is corrupted", hdev->name);
> err = -EINVAL;
> goto done;
> }
> @@ -156,7 +152,6 @@ int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
> msleep(250);
>
> done:
> - release_firmware(fw);
> return err;
> }
> EXPORT_SYMBOL(btbcm_patchram);
> @@ -168,6 +163,7 @@ static int btbcm_reset(struct hci_dev *hdev)
> skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
> if (IS_ERR(skb)) {
> int err = PTR_ERR(skb);
> +
> BT_ERR("%s: BCM: Reset failed (%d)", hdev->name, err);
> return err;
> }
> @@ -242,9 +238,101 @@ static const struct {
> const char *name;
> } bcm_uart_subver_table[] = {
> { 0x410e, "BCM43341B0" }, /* 002.001.014 */
> + { 0x4406, "BCM4324B3" }, /* 002.004.006 */
> + { 0x610c, "BCM4354_003.001.012.0306.0659"}, /* 003.001.012 */

Please just BCM4354 here.

And I have initial patches for the manifest file to map modalias to firmware names. That will then hopefully solve all of these and we can just update this easily from userspace. I will send patches as soon as I have cleaned them up a bit.

Regards

Marcel


2015-06-04 23:46:11

by Ilya Faenson

[permalink] [raw]
Subject: RE: [PATCH 0/5] Broadcom Bluetooth UART device driver

Hi Fred,

Appreciate you giving this code a try.

This code includes a platform device. It needs to be instantiated. My customer's platform is ARM so I configure a Device Tree binding to instantiate a device (see the first patch on how it should be done). The binding must have the compatible id that the driver lists. That binding should also have all the relevant device parameters including the tty it is configured on ("ttyS4" in your case). The tty is used to map the BlueZ protocol instance into the platform device instance. We would not need it if multiple BT UART devices were not to be supported. It is Marcel's requirement to support multiple devices however. Now, your platform may be ACPI so you would then need to instantiate a device on the ACPI device node. I don't have Linux running on the ACPI hardware at the moment so I've not tried adapting this code to ACPI. Would be great if you could help us with that. :-) You do know all the parameters for the device: I've specified them to you on a separate thread.

Looking forward to further progress!
-Ilya

-----Original Message-----
From: Frederic Danis [mailto:[email protected]]
Sent: Thursday, June 04, 2015 12:01 PM
To: Ilya Faenson
Cc: Marcel Holtmann; [email protected]
Subject: Re: [PATCH 0/5] Broadcom Bluetooth UART device driver

Hello Ilya,

On 03/06/2015 23:01, Ilya Faenson wrote:
> This is a merge of the Broadcom Bluetooth UART logic with
> the latest line discipline and protocol enhancements implemented
> by Frederic Danis of Intel.
>
> Ilya Faenson (5):
> Broadcom Bluetooth UART Device Tree bindings
> Intel based H4 line discipline enhancements
> Broadcom Bluetooth UART Platform Driver
> Broadcom Bluetooth protocol UART support
> BlueZ Broadcom UART Protocol
>
> .../devicetree/bindings/net/bluetooth/btbcm.txt | 82 +++
> drivers/bluetooth/Kconfig | 9 +
> drivers/bluetooth/Makefile | 1 +
> drivers/bluetooth/btbcm.c | 142 ++++-
> drivers/bluetooth/btbcm.h | 21 +-
> drivers/bluetooth/btbcm_uart.c | 673 +++++++++++++++++++++
> drivers/bluetooth/btbcm_uart.h | 89 +++
> drivers/bluetooth/hci_bcm.c | 481 ++++++++++++++-
> drivers/bluetooth/hci_ldisc.c | 110 +++-
> drivers/bluetooth/hci_uart.h | 6 +
> 10 files changed, 1565 insertions(+), 49 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
> create mode 100755 drivers/bluetooth/btbcm_uart.c
> create mode 100755 drivers/bluetooth/btbcm_uart.h
>

I applied your patches on top of mines and test them on a T100, it did
not work as expected with following traces:

Jun 4 17:41:43 fdanis-T100TA kernel: [ 125.409680] Bluetooth: HCI UART
driver ver 2.3
Jun 4 17:41:43 fdanis-T100TA kernel: [ 125.409688] Bluetooth: HCI UART
protocol H4 registered
Jun 4 17:41:43 fdanis-T100TA kernel: [ 125.409691] Bluetooth: HCI UART
protocol BCSP registered
Jun 4 17:41:43 fdanis-T100TA kernel: [ 125.409693] Bluetooth: HCI UART
protocol LL registered
Jun 4 17:41:43 fdanis-T100TA kernel: [ 125.409696] Bluetooth: HCI UART
protocol ATH3K registered
Jun 4 17:41:43 fdanis-T100TA kernel: [ 125.409698] Bluetooth: HCI UART
protocol Three-wire (H5) registered
Jun 4 17:41:43 fdanis-T100TA kernel: [ 125.409701] Bluetooth: HCI UART
protocol BCM registered
Jun 4 17:42:00 fdanis-T100TA kernel: [ 142.130205] NET: Registered
protocol family 38
Jun 4 17:43:21 fdanis-T100TA kernel: [ 223.397482] hci_uart_tty_open:
tty ffff880074900000
Jun 4 17:43:21 fdanis-T100TA kernel: [ 223.397551] hci_uart_tty_ioctl:
Jun 4 17:43:21 fdanis-T100TA kernel: [ 223.397556] hci_uart_tty_ioctl:
Jun 4 17:43:21 fdanis-T100TA kernel: [ 223.397560] bcm_open: bcm_open
hu ffff880036683d80
Jun 4 17:43:21 fdanis-T100TA kernel: [ 223.397567] btbcm_uart_control:
btbcm_uart_control - configure callbacks
Jun 4 17:43:21 fdanis-T100TA kernel: [ 223.397572] btbcm_uart_control:
btbcm_uart_control - configure callbacks for ttyS4(ffff880036683d80)
Jun 4 17:43:21 fdanis-T100TA kernel: [ 223.397575] btbcm_uart_control:
btbcm_uart_control - no device!
Jun 4 17:43:21 fdanis-T100TA kernel: [ 223.397579] bcm_open: bcm_open
failed to set driver callbacks -2
Jun 4 17:43:21 fdanis-T100TA kernel: [ 223.397801] hci_uart_tty_close:
tty ffff880074900000

Regards

Fred

--
Frederic Danis Open Source Technology Center
[email protected] Intel Corporation


2015-06-04 16:00:36

by Frederic Danis

[permalink] [raw]
Subject: Re: [PATCH 0/5] Broadcom Bluetooth UART device driver

Hello Ilya,

On 03/06/2015 23:01, Ilya Faenson wrote:
> This is a merge of the Broadcom Bluetooth UART logic with
> the latest line discipline and protocol enhancements implemented
> by Frederic Danis of Intel.
>
> Ilya Faenson (5):
> Broadcom Bluetooth UART Device Tree bindings
> Intel based H4 line discipline enhancements
> Broadcom Bluetooth UART Platform Driver
> Broadcom Bluetooth protocol UART support
> BlueZ Broadcom UART Protocol
>
> .../devicetree/bindings/net/bluetooth/btbcm.txt | 82 +++
> drivers/bluetooth/Kconfig | 9 +
> drivers/bluetooth/Makefile | 1 +
> drivers/bluetooth/btbcm.c | 142 ++++-
> drivers/bluetooth/btbcm.h | 21 +-
> drivers/bluetooth/btbcm_uart.c | 673 +++++++++++++++++++++
> drivers/bluetooth/btbcm_uart.h | 89 +++
> drivers/bluetooth/hci_bcm.c | 481 ++++++++++++++-
> drivers/bluetooth/hci_ldisc.c | 110 +++-
> drivers/bluetooth/hci_uart.h | 6 +
> 10 files changed, 1565 insertions(+), 49 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
> create mode 100755 drivers/bluetooth/btbcm_uart.c
> create mode 100755 drivers/bluetooth/btbcm_uart.h
>

I applied your patches on top of mines and test them on a T100, it did
not work as expected with following traces:

Jun 4 17:41:43 fdanis-T100TA kernel: [ 125.409680] Bluetooth: HCI UART
driver ver 2.3
Jun 4 17:41:43 fdanis-T100TA kernel: [ 125.409688] Bluetooth: HCI UART
protocol H4 registered
Jun 4 17:41:43 fdanis-T100TA kernel: [ 125.409691] Bluetooth: HCI UART
protocol BCSP registered
Jun 4 17:41:43 fdanis-T100TA kernel: [ 125.409693] Bluetooth: HCI UART
protocol LL registered
Jun 4 17:41:43 fdanis-T100TA kernel: [ 125.409696] Bluetooth: HCI UART
protocol ATH3K registered
Jun 4 17:41:43 fdanis-T100TA kernel: [ 125.409698] Bluetooth: HCI UART
protocol Three-wire (H5) registered
Jun 4 17:41:43 fdanis-T100TA kernel: [ 125.409701] Bluetooth: HCI UART
protocol BCM registered
Jun 4 17:42:00 fdanis-T100TA kernel: [ 142.130205] NET: Registered
protocol family 38
Jun 4 17:43:21 fdanis-T100TA kernel: [ 223.397482] hci_uart_tty_open:
tty ffff880074900000
Jun 4 17:43:21 fdanis-T100TA kernel: [ 223.397551] hci_uart_tty_ioctl:
Jun 4 17:43:21 fdanis-T100TA kernel: [ 223.397556] hci_uart_tty_ioctl:
Jun 4 17:43:21 fdanis-T100TA kernel: [ 223.397560] bcm_open: bcm_open
hu ffff880036683d80
Jun 4 17:43:21 fdanis-T100TA kernel: [ 223.397567] btbcm_uart_control:
btbcm_uart_control - configure callbacks
Jun 4 17:43:21 fdanis-T100TA kernel: [ 223.397572] btbcm_uart_control:
btbcm_uart_control - configure callbacks for ttyS4(ffff880036683d80)
Jun 4 17:43:21 fdanis-T100TA kernel: [ 223.397575] btbcm_uart_control:
btbcm_uart_control - no device!
Jun 4 17:43:21 fdanis-T100TA kernel: [ 223.397579] bcm_open: bcm_open
failed to set driver callbacks -2
Jun 4 17:43:21 fdanis-T100TA kernel: [ 223.397801] hci_uart_tty_close:
tty ffff880074900000

Regards

Fred

--
Frederic Danis Open Source Technology Center
[email protected] Intel Corporation

2015-06-04 11:13:44

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH 5/5] BlueZ Broadcom UART Protocol

On 06/04/15 12:14, Frederic Danis wrote:
> On 03/06/2015 23:01, Ilya Faenson wrote:
>> Merge Broadcom protocol with the Intel implementation, providing
>> for UART setup, firmware download and power management.
>>
>> Signed-off-by: Ilya Faenson <[email protected]>
>> ---
>> drivers/bluetooth/hci_bcm.c | 481
>> ++++++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 466 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
>> index 1ec0b4a..2894253 100644
>> --- a/drivers/bluetooth/hci_bcm.c
>> +++ b/drivers/bluetooth/hci_bcm.c
>> @@ -3,6 +3,7 @@
>> * Bluetooth HCI UART driver for Broadcom devices
>> *
>> * Copyright (C) 2015 Intel Corporation
>> + * Copyright (C) 2015 Broadcom Corporation
>> *
>> *
>> * This program is free software; you can redistribute it and/or modify
>> @@ -24,45 +25,355 @@
>> #include <linux/kernel.h>
>> #include <linux/errno.h>
>> #include <linux/skbuff.h>
>> +#include <linux/tty.h>
>>
>> #include <net/bluetooth/bluetooth.h>
>> #include <net/bluetooth/hci_core.h>
>>
>> #include "btbcm.h"
>> #include "hci_uart.h"
>> +#include "btbcm_uart.h"
>> +
>> +#define BCM43XX_CLOCK_48 1
>> +#define BCM43XX_CLOCK_24 2
>>
>> struct bcm_data {
>> struct sk_buff *rx_skb;
>> struct sk_buff_head txq;
>> + struct hci_uart *hu;
>> +
>> + bool is_suspended; /* suspend/resume flag */
>> +
>> + struct timer_list timer; /* idle timer */
>> +
>> + struct btbcm_uart_parameters pars; /* device parameters */
>> + void *device_context; /* ACPI/DT device context */
>> };
>>
>> +/* Suspend/resume synchronization mutex */
>> +static DEFINE_MUTEX(plock);
>> +
>> +/* Forward reference */
>> +static struct hci_uart_proto bcm_proto;
>> +
>> +/*
>> + * Callbacks from the BCMBT_UART device
>> + */
>> +
>> +/*
>> + * The platform is suspending. Stop UART activity
>> + */
>> +static void suspend_notification(void *context)
>> +{
>> + struct hci_uart *hu = (struct hci_uart *)context;
>> + struct bcm_data *h4 = hu->priv;
>> +
>> + BT_DBG("%s: is_suspended %d", __func__, h4->is_suspended);
>
> You do not need to include the function name in BT_DBG, this is managed
> by dynamic debug feature +f flag when enabling debug traces.

Provided your kernel is built with dynamic debug enabled. ;-)

Regards,
Arend

> Regards
>
> Fred
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


2015-06-04 10:14:43

by Frederic Danis

[permalink] [raw]
Subject: Re: [PATCH 5/5] BlueZ Broadcom UART Protocol

On 03/06/2015 23:01, Ilya Faenson wrote:
> Merge Broadcom protocol with the Intel implementation, providing
> for UART setup, firmware download and power management.
>
> Signed-off-by: Ilya Faenson <[email protected]>
> ---
> drivers/bluetooth/hci_bcm.c | 481 ++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 466 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
> index 1ec0b4a..2894253 100644
> --- a/drivers/bluetooth/hci_bcm.c
> +++ b/drivers/bluetooth/hci_bcm.c
> @@ -3,6 +3,7 @@
> * Bluetooth HCI UART driver for Broadcom devices
> *
> * Copyright (C) 2015 Intel Corporation
> + * Copyright (C) 2015 Broadcom Corporation
> *
> *
> * This program is free software; you can redistribute it and/or modify
> @@ -24,45 +25,355 @@
> #include <linux/kernel.h>
> #include <linux/errno.h>
> #include <linux/skbuff.h>
> +#include <linux/tty.h>
>
> #include <net/bluetooth/bluetooth.h>
> #include <net/bluetooth/hci_core.h>
>
> #include "btbcm.h"
> #include "hci_uart.h"
> +#include "btbcm_uart.h"
> +
> +#define BCM43XX_CLOCK_48 1
> +#define BCM43XX_CLOCK_24 2
>
> struct bcm_data {
> struct sk_buff *rx_skb;
> struct sk_buff_head txq;
> + struct hci_uart *hu;
> +
> + bool is_suspended; /* suspend/resume flag */
> +
> + struct timer_list timer; /* idle timer */
> +
> + struct btbcm_uart_parameters pars; /* device parameters */
> + void *device_context; /* ACPI/DT device context */
> };
>
> +/* Suspend/resume synchronization mutex */
> +static DEFINE_MUTEX(plock);
> +
> +/* Forward reference */
> +static struct hci_uart_proto bcm_proto;
> +
> +/*
> + * Callbacks from the BCMBT_UART device
> + */
> +
> +/*
> + * The platform is suspending. Stop UART activity
> + */
> +static void suspend_notification(void *context)
> +{
> + struct hci_uart *hu = (struct hci_uart *)context;
> + struct bcm_data *h4 = hu->priv;
> +
> + BT_DBG("%s: is_suspended %d", __func__, h4->is_suspended);

You do not need to include the function name in BT_DBG, this is managed
by dynamic debug feature +f flag when enabling debug traces.

Regards

Fred

2015-06-04 08:41:55

by Frederic Danis

[permalink] [raw]
Subject: Re: [PATCH 3/5] Broadcom Bluetooth UART Platform Driver

Hello Ilya,

On 03/06/2015 23:01, Ilya Faenson wrote:
> Introduce the device tree enumerated Broadcom Bluetooth UART driver.
>
> Signed-off-by: Ilya Faenson <[email protected]>
> ---
> drivers/bluetooth/Kconfig | 9 +
> drivers/bluetooth/Makefile | 1 +
> drivers/bluetooth/btbcm_uart.c | 673 +++++++++++++++++++++++++++++++++++++++++
> drivers/bluetooth/btbcm_uart.h | 89 ++++++
> 4 files changed, 772 insertions(+)
> create mode 100644 drivers/bluetooth/btbcm_uart.c
> create mode 100644 drivers/bluetooth/btbcm_uart.h
>
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index 2e77707..5eee3ed 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -143,6 +143,7 @@ config BT_HCIUART_BCM
> bool "Broadcom protocol support"
> depends on BT_HCIUART
> select BT_HCIUART_H4
> + select BT_UART_BCM
> select BT_BCM
> help
> The Broadcom protocol support enables Bluetooth HCI over serial
> @@ -150,6 +151,14 @@ config BT_HCIUART_BCM
>
> Say Y here to compile support for Broadcom protocol.
>
> +config BT_UART_BCM
> + tristate "Broadcom BT UART driver"
> + depends on BT_HCIUART_H4 && TTY
> + help
> + This driver supports the HCI_UART_BT protocol.
> +
> + It manages Bluetooth UART device properties and GPIOs.
> +
> config BT_HCIBCM203X
> tristate "HCI BCM203x USB driver"
> depends on USB
> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> index f40e194..e908a88 100644
> --- a/drivers/bluetooth/Makefile
> +++ b/drivers/bluetooth/Makefile
> @@ -21,6 +21,7 @@ obj-$(CONFIG_BT_MRVL) += btmrvl.o
> obj-$(CONFIG_BT_MRVL_SDIO) += btmrvl_sdio.o
> obj-$(CONFIG_BT_WILINK) += btwilink.o
> obj-$(CONFIG_BT_BCM) += btbcm.o
> +obj-$(CONFIG_BT_UART_BCM) += btbcm_uart.o
> obj-$(CONFIG_BT_RTL) += btrtl.o
>
> btmrvl-y := btmrvl_main.o
> diff --git a/drivers/bluetooth/btbcm_uart.c b/drivers/bluetooth/btbcm_uart.c
> new file mode 100644
> index 0000000..ccd02a9
> --- /dev/null
> +++ b/drivers/bluetooth/btbcm_uart.c
> @@ -0,0 +1,673 @@
> +/*
> + * Bluetooth BCM UART Driver
> + *
> + * Copyright (c) 2015 Broadcom Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/module.h>
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/types.h>
> +#include <linux/fcntl.h>
> +#include <linux/interrupt.h>
> +#include <linux/ptrace.h>
> +#include <linux/poll.h>
> +
> +#include <linux/slab.h>
> +#include <linux/tty.h>
> +#include <linux/errno.h>
> +#include <linux/string.h>
> +#include <linux/signal.h>
> +#include <linux/ioctl.h>
> +#include <linux/skbuff.h>
> +#include <linux/list.h>
> +
> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +
> +#include <linux/gpio/consumer.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>
> +#include <linux/of_platform.h>
> +
> +#include "btbcm_uart.h"
> +
> +static int idle_timeout = 5;
> +module_param(idle_timeout, int, 0);
> +MODULE_PARM_DESC(idle_timeout, "Bluetooth idle timeout in seconds");
> +
> +/* Device context */
> +struct bcm_device {
> + struct list_head list;
> +
> + struct platform_device *pdev;
> + struct gpio_desc *bt_wake_gpio;
> + struct gpio_desc *dev_wake_gpio;
> + struct gpio_desc *reg_on_gpio;
> + int bt_wake_irq;
> + int dev_wake_active_low;
> + int reg_on_active_low;
> + int bt_wake_active_low;
> + bool configure_sleep;
> + u32 manual_fc;
> + u32 oper_speed;
> + bool configure_audio;
> + u32 pcm_clockmode;
> + u32 pcm_fillmethod;
> + u32 pcm_fillnum;
> + u32 pcm_fillvalue;
> + u32 pcm_incallbitclock;
> + u32 pcm_lsbfirst;
> + u32 pcm_rightjustify;
> + u32 pcm_routing;
> + u32 pcm_shortframesync;
> + u32 pcm_syncmode;
> +
> + char tty_name[64];
> +
> + struct btbcm_uart_callbacks protocol_callbacks;
> + struct work_struct wakeup_work;
> +};
> +
> +/* List of BCM BT UART devices */
> +static DEFINE_SPINLOCK(device_list_lock);
> +static LIST_HEAD(device_list);
> +
> +/*
> + * Calling the BCM protocol at lower execution priority
> + */
> +static void bcm_bt_wakeup_task(struct work_struct *ws)
> +{
> + int gpio_value;
> + struct bcm_device *p_bcm_device =
> + container_of(ws, struct bcm_device, wakeup_work);
> +
> + if (!p_bcm_device) {
> + BT_DBG("%s - failing, no device", __func__);

You do not need to include the function name in BT_DBG, this is managed
by dynamic debug feature +f flag when enabling debug traces.

Regards

Fred

2015-06-04 07:44:36

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH 0/5] Broadcom Bluetooth UART device driver

On 06/03/15 23:01, Ilya Faenson wrote:
> This is a merge of the Broadcom Bluetooth UART logic with
> the latest line discipline and protocol enhancements implemented
> by Frederic Danis of Intel.

The series from Frederic Danis can be found by its message-id:

[email protected]

Regards,
Arend

[1]
http://mid.gmane.org/[email protected]

> Ilya Faenson (5):
> Broadcom Bluetooth UART Device Tree bindings
> Intel based H4 line discipline enhancements
> Broadcom Bluetooth UART Platform Driver
> Broadcom Bluetooth protocol UART support
> BlueZ Broadcom UART Protocol
>
> .../devicetree/bindings/net/bluetooth/btbcm.txt | 82 +++
> drivers/bluetooth/Kconfig | 9 +
> drivers/bluetooth/Makefile | 1 +
> drivers/bluetooth/btbcm.c | 142 ++++-
> drivers/bluetooth/btbcm.h | 21 +-
> drivers/bluetooth/btbcm_uart.c | 673 +++++++++++++++++++++
> drivers/bluetooth/btbcm_uart.h | 89 +++
> drivers/bluetooth/hci_bcm.c | 481 ++++++++++++++-
> drivers/bluetooth/hci_ldisc.c | 110 +++-
> drivers/bluetooth/hci_uart.h | 6 +
> 10 files changed, 1565 insertions(+), 49 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
> create mode 100755 drivers/bluetooth/btbcm_uart.c
> create mode 100755 drivers/bluetooth/btbcm_uart.h
>


2015-06-03 21:01:44

by Ilya Faenson

[permalink] [raw]
Subject: [PATCH 5/5] BlueZ Broadcom UART Protocol

Merge Broadcom protocol with the Intel implementation, providing
for UART setup, firmware download and power management.

Signed-off-by: Ilya Faenson <[email protected]>
---
drivers/bluetooth/hci_bcm.c | 481 ++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 466 insertions(+), 15 deletions(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 1ec0b4a..2894253 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -3,6 +3,7 @@
* Bluetooth HCI UART driver for Broadcom devices
*
* Copyright (C) 2015 Intel Corporation
+ * Copyright (C) 2015 Broadcom Corporation
*
*
* This program is free software; you can redistribute it and/or modify
@@ -24,45 +25,355 @@
#include <linux/kernel.h>
#include <linux/errno.h>
#include <linux/skbuff.h>
+#include <linux/tty.h>

#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>

#include "btbcm.h"
#include "hci_uart.h"
+#include "btbcm_uart.h"
+
+#define BCM43XX_CLOCK_48 1
+#define BCM43XX_CLOCK_24 2

struct bcm_data {
struct sk_buff *rx_skb;
struct sk_buff_head txq;
+ struct hci_uart *hu;
+
+ bool is_suspended; /* suspend/resume flag */
+
+ struct timer_list timer; /* idle timer */
+
+ struct btbcm_uart_parameters pars; /* device parameters */
+ void *device_context; /* ACPI/DT device context */
};

+/* Suspend/resume synchronization mutex */
+static DEFINE_MUTEX(plock);
+
+/* Forward reference */
+static struct hci_uart_proto bcm_proto;
+
+/*
+ * Callbacks from the BCMBT_UART device
+ */
+
+/*
+ * The platform is suspending. Stop UART activity
+ */
+static void suspend_notification(void *context)
+{
+ struct hci_uart *hu = (struct hci_uart *)context;
+ struct bcm_data *h4 = hu->priv;
+
+ BT_DBG("%s: is_suspended %d", __func__, h4->is_suspended);
+
+ if (!h4->pars.configure_sleep)
+ return;
+
+ if (!h4->is_suspended) {
+ if (h4->pars.manual_fc)
+ hci_uart_flow_control_device(hu);
+
+ /* Once this callback returns, driver suspends BT via GPIO */
+ h4->is_suspended = true;
+ }
+}
+
+/*
+ * The platform is resuming. Resume UART activity.
+ */
+static void resume_notification(void *context)
+{
+ struct hci_uart *hu = (struct hci_uart *)context;
+ struct bcm_data *h4 = hu->priv;
+
+ BT_DBG("%s: is_suspended %d", __func__, h4->is_suspended);
+
+ if (!h4->pars.configure_sleep)
+ return;
+
+ /* When this callback executes, the device has woken up already */
+ if (h4->is_suspended) {
+ h4->is_suspended = false;
+
+ if (h4->pars.manual_fc)
+ hci_uart_unflow_control_device(hu);
+ }
+
+ /* If we're resumed, the idle timer must be running */
+ mod_timer(&h4->timer, jiffies +
+ msecs_to_jiffies(h4->pars.idle_timeout_in_secs * 1000));
+}
+
+/*
+ * The BT device is resuming. Resume UART activity if suspended
+ */
+static void wakeup_notification(void *context)
+{
+ struct hci_uart *hu = (struct hci_uart *)context;
+ struct bcm_data *h4 = hu->priv;
+
+ BT_DBG("%s: is_suspended %d", __func__, h4->is_suspended);
+
+ if (!h4->pars.configure_sleep)
+ return;
+
+ if (h4->is_suspended) {
+ if (h4->pars.manual_fc)
+ hci_uart_unflow_control_device(hu);
+
+ h4->is_suspended = false;
+ }
+
+ /* If we're resumed, the idle timer must be running */
+ mod_timer(&h4->timer, jiffies + msecs_to_jiffies(
+ h4->pars.idle_timeout_in_secs * 1000));
+}
+
+/*
+ * Make sure we're awake
+ * (called when the resumed state is required)
+ */
+static void bcm_ensure_wakeup(struct hci_uart *hu)
+{
+ struct bcm_data *h4 = hu->priv;
+ int status;
+
+ if (!h4->pars.configure_sleep)
+ return;
+
+ /* Suspend/resume operations are serialized */
+ mutex_lock(&plock);
+
+ /* Nothing to do if resumed already */
+ if (!h4->is_suspended) {
+ mutex_unlock(&plock);
+
+ /* Just reset the timer */
+ status = mod_timer(&h4->timer, jiffies + msecs_to_jiffies(
+ h4->pars.idle_timeout_in_secs * 1000));
+ return;
+ }
+
+ /* Wakeup the device */
+ status = btbcm_uart_control(BTBCM_UART_ACTION_RESUME,
+ h4->device_context, NULL, NULL);
+ if (status)
+ BT_DBG("%s: failed to resume driver %d", __func__, status);
+
+ /* Unflow control the port if configured */
+ resume_notification(hu);
+
+ mutex_unlock(&plock);
+}
+
+/*
+ * Idle timer callback
+ */
+static void bcm_idle_timeout(unsigned long arg)
+{
+ struct hci_uart *hu = (struct hci_uart *)arg;
+ struct bcm_data *h4 = hu->priv;
+ int status;
+
+ BT_DBG("%s: hu %p", __func__, hu);
+
+ /* Suspend/resume operations are serialized */
+ mutex_lock(&plock);
+
+ if (!h4->is_suspended) {
+ /* Flow control the port if configured */
+ suspend_notification(hu);
+
+ /* Suspend the device */
+ status = btbcm_uart_control(BTBCM_UART_ACTION_SUSPEND,
+ h4->device_context, NULL, NULL);
+ if (status)
+ BT_DBG("%s: failed to suspend device %d", __func__,
+ status);
+ }
+
+ mutex_unlock(&plock);
+}
+
+struct hci_cp_bcm_set_speed {
+ __le16 dummy;
+ __le32 speed;
+} __packed;
+
+static int bcm_set_baudrate(struct hci_uart *hu, unsigned int speed)
+{
+ struct hci_dev *hdev = hu->hdev;
+ struct sk_buff *skb;
+ struct hci_cp_bcm_set_speed param = { 0, cpu_to_le32(speed) };
+
+ if (speed > 3000000) {
+ u8 clock = BCM43XX_CLOCK_48;
+
+ BT_DBG("%s: Set Controller clock (%d)", hdev->name, clock);
+
+ skb = __hci_cmd_sync(hdev, 0xfc45, 1, &clock, HCI_INIT_TIMEOUT);
+ if (IS_ERR(skb)) {
+ BT_ERR("%s: failed to write update clock command (%ld)",
+ hdev->name, PTR_ERR(skb));
+ return PTR_ERR(skb);
+ }
+
+ kfree_skb(skb);
+ }
+
+ BT_DBG("%s: Set Controller UART speed to %d bit/s", hdev->name, speed);
+
+ skb = __hci_cmd_sync(hdev, 0xfc18, sizeof(param), &param,
+ HCI_INIT_TIMEOUT);
+ if (IS_ERR(skb)) {
+ BT_ERR("%s: failed to write update baudrate command (%ld)",
+ hdev->name, PTR_ERR(skb));
+ return PTR_ERR(skb);
+ }
+
+ kfree_skb(skb);
+
+ return 0;
+}
+
static int bcm_open(struct hci_uart *hu)
{
- struct bcm_data *bcm;
+ struct btbcm_uart_callbacks callbacks;
+ unsigned long callbacks_size = sizeof(callbacks);
+ int status;
+ struct bcm_data *h4;
+ struct tty_struct *tty = hu->tty;

- BT_DBG("hu %p", hu);
+ BT_DBG("bcm_open hu %p", hu);

- bcm = kzalloc(sizeof(*bcm), GFP_KERNEL);
- if (!bcm)
+ h4 = kzalloc(sizeof(*h4), GFP_KERNEL);
+ if (!h4)
return -ENOMEM;

- skb_queue_head_init(&bcm->txq);
+ skb_queue_head_init(&h4->txq);
+ hu->priv = h4;
+ h4->hu = hu;
+ h4->is_suspended = false;
+
+ /* Configure callbacks on the driver */
+ callbacks.interface_version = BTBCM_UART_INTERFACE_VERSION;
+ callbacks.context = hu;
+ strcpy(callbacks.name, tty->name);
+ callbacks.p_suspend = suspend_notification;
+ callbacks.p_resume = resume_notification;
+ callbacks.p_wakeup = wakeup_notification;
+ status = btbcm_uart_control(BTBCM_UART_ACTION_CONFIGURE_CALLBACKS,
+ NULL, &callbacks, &callbacks_size);
+ if (status) {
+ BT_DBG("bcm_open failed to set driver callbacks %d", status);
+ return status;
+ }
+ if (callbacks_size != sizeof(void *)) {
+ BT_DBG("bcm_open got back %d bytes from callbacks?!",
+ (int)callbacks_size);
+ return -EMSGSIZE;
+ }
+ memcpy(&h4->device_context, &callbacks, sizeof(void *));
+ BT_DBG("bcm_open callbacks context %p", h4->device_context);
+
+ /* Retrieve device parameters */
+ callbacks_size = sizeof(h4->pars);
+ status = btbcm_uart_control(BTBCM_UART_ACTION_GET_PARAMETERS,
+ h4->device_context, &h4->pars,
+ &callbacks_size);
+ if (status) {
+ BT_DBG("bcm_open failed to get dev parameters %d", status);
+ return status;
+ }
+ BT_DBG("Pars ver %d csleep %d dalow %d balow %d idle %d",
+ h4->pars.interface_version, h4->pars.configure_sleep,
+ h4->pars.dev_wake_active_low, h4->pars.bt_wake_active_low,
+ h4->pars.idle_timeout_in_secs);
+ bcm_proto.oper_speed = h4->pars.oper_speed;
+
+ /* Cycle power to make sure the device is in the known state */
+ status = btbcm_uart_control(BTBCM_UART_ACTION_POWER_OFF,
+ h4->device_context, NULL, NULL);
+ if (status) {
+ BT_DBG("bcm_open failed to power off %d", status);
+ } else {
+ status = btbcm_uart_control(BTBCM_UART_ACTION_POWER_ON,
+ h4->device_context, NULL, NULL);
+ if (status)
+ BT_DBG("bcm_open failed to power on %d", status);
+ }
+
+ /* Start the idle timer */
+ if (h4->pars.configure_sleep) {
+ setup_timer(&h4->timer, bcm_idle_timeout, (unsigned long)hu);
+ if (h4->pars.configure_sleep)
+ mod_timer(&h4->timer, jiffies + msecs_to_jiffies(
+ h4->pars.idle_timeout_in_secs * 1000));
+ }

- hu->priv = bcm;
return 0;
}

static int bcm_close(struct hci_uart *hu)
{
- struct bcm_data *bcm = hu->priv;
+ struct btbcm_uart_callbacks callbacks;
+ unsigned long callbacks_size = sizeof(callbacks);
+ struct bcm_data *h4 = hu->priv;
+ int status;

- BT_DBG("hu %p", hu);
+ hu->priv = NULL;

- skb_queue_purge(&bcm->txq);
- kfree_skb(bcm->rx_skb);
- kfree(bcm);
+ BT_DBG("bcm_close hu %p", hu);
+
+ /* If we're being closed, we must suspend */
+ if (h4->pars.configure_sleep) {
+ mutex_lock(&plock);
+
+ if (!h4->is_suspended) {
+ /* Flow control the port */
+ suspend_notification(hu);
+
+ /* Suspend the device */
+ status = btbcm_uart_control(BTBCM_UART_ACTION_SUSPEND,
+ h4->device_context, NULL,
+ NULL);
+ if (status) {
+ BT_DBG("bcm_close suspend driver fail %d",
+ status);
+ }
+ }
+
+ mutex_unlock(&plock);
+
+ del_timer_sync(&h4->timer);
+ }
+
+ /* Power off the device if possible */
+ status = btbcm_uart_control(BTBCM_UART_ACTION_POWER_OFF,
+ h4->device_context, NULL, NULL);
+ if (status)
+ BT_DBG("bcm_close failed to power off %d", status);
+
+ /* de-configure callbacks on the driver */
+ callbacks.interface_version = BTBCM_UART_INTERFACE_VERSION;
+ callbacks.context = hu;
+ callbacks.p_suspend = NULL;
+ callbacks.p_resume = NULL;
+ callbacks.p_wakeup = NULL;
+ status = btbcm_uart_control(BTBCM_UART_ACTION_CONFIGURE_CALLBACKS,
+ h4->device_context, &callbacks,
+ &callbacks_size);
+ if (status)
+ BT_DBG("bcm_close failed to reset drv callbacks %d", status);
+ skb_queue_purge(&h4->txq);

hu->priv = NULL;
+ kfree(h4);
+
return 0;
}

@@ -79,11 +390,139 @@ static int bcm_flush(struct hci_uart *hu)

static int bcm_setup(struct hci_uart *hu)
{
- BT_DBG("hu %p", hu);
+ char fw_name[128];
+ const struct firmware *fw;
+ int err;
+ struct sk_buff *skb;
+ struct bcm_data *h4 = hu->priv;
+ unsigned char sleep_pars[] = {
+ 0x01, /* sleep mode 1=UART */
+ 0x02, /* idle threshold HOST (value * 300ms) */
+ 0x02, /* idle threshold HC (value * 300ms) */
+ 0x01, /* BT_WAKE active mode - 1=active high, 0 = active low */
+ 0x00, /* HOST_WAKE active mode - 1=active high, 0 = active low */
+ 0x01, /* Allow host sleep during SCO - FALSE */
+ 0x01, /* combine sleep mode and LPM - 1 == TRUE */
+ 0x00, /* enable tristate control of UART TX line - FALSE */
+ 0x00, /* USB auto-sleep on USB SUSPEND */
+ 0x00, /* USB USB RESUME timeout (seconds) */
+ 0x00, /* Pulsed Host Wake */
+ 0x00 /* Enable Break To Host */
+ };
+ unsigned char pcm_int_pars[] = {
+ 0x00, /* 0=PCM routing, 1=SCO over HCI */
+ 0x02, /* 0=128Kbps,1=256Kbps,2=512Kbps,3=1024Kbps,4=2048Kbps */
+ 0x00, /* short frame sync 0=short, 1=long */
+ 0x00, /* sync mode 0=slave, 1=master */
+ 0x00 /* clock mode 0=slave, 1=master */
+ };
+ unsigned char pcm_format_pars[] = {
+ 0x00, /* LSB_First 1=TRUE, 0=FALSE */
+ 0x00, /* Fill_Value (use 0-7 for fill bits value) */
+ 0x02, /* Fill_Method (2=sign extended) */
+ 0x03, /* Fill_Num # of fill bits 0-3) */
+ 0x01 /* Right_Justify 1=TRUE, 0=FALSE */
+ };
+ unsigned char time_slot_number = 0;
+
+ BT_DBG("%s: hu %p", __func__, hu);

hu->hdev->set_bdaddr = btbcm_set_bdaddr;

- return btbcm_setup_patchram(hu->hdev);
+ err = btbcm_initialize(hu->hdev, fw_name, sizeof(fw_name));
+ if (err)
+ return err;
+
+ err = request_firmware(&fw, fw_name, &hu->hdev->dev);
+ if (err < 0) {
+ BT_INFO("%s: BCM: Patch %s not found", hu->hdev->name, fw_name);
+ return 0;
+ }
+
+ err = btbcm_patchram(hu->hdev, fw);
+ if (err) {
+ BT_INFO("%s: BCM: Patch failed (%d)", hu->hdev->name, err);
+ goto finalize;
+ }
+
+ if (hu->proto->init_speed)
+ hci_uart_set_baudrate(hu, hu->proto->init_speed);
+
+ if (hu->proto->oper_speed) {
+ err = bcm_set_baudrate(hu, hu->proto->oper_speed);
+ if (!err)
+ /* hci_uart_set_baudrate() has no return value as
+ tty_set_termios() return is always 0 */
+ hci_uart_set_baudrate(hu, hu->proto->oper_speed);
+ }
+
+ /* Configure SCO PCM parameters */
+ if (h4->pars.configure_audio) {
+ pcm_int_pars[0] = h4->pars.pcm_routing;
+ pcm_int_pars[1] = h4->pars.pcm_incallbitclock;
+ pcm_int_pars[2] = h4->pars.pcm_shortframesync;
+ pcm_int_pars[3] = h4->pars.pcm_syncmode;
+ pcm_int_pars[4] = h4->pars.pcm_clockmode;
+ skb = __hci_cmd_sync(hu->hdev, 0xfc1c, sizeof(pcm_int_pars),
+ pcm_int_pars, HCI_INIT_TIMEOUT);
+ if (IS_ERR(skb)) {
+ err = PTR_ERR(skb);
+ BT_ERR("bcm_setup pcm_ INT VSC failed (%d)", err);
+ goto finalize;
+ }
+ kfree_skb(skb);
+ BT_DBG("bcm_setup pcm_ INT Parameters VSC succeeded");
+
+ pcm_format_pars[0] = h4->pars.pcm_lsbfirst;
+ pcm_format_pars[1] = h4->pars.pcm_fillvalue;
+ pcm_format_pars[2] = h4->pars.pcm_fillmethod;
+ pcm_format_pars[3] = h4->pars.pcm_fillnum;
+ pcm_format_pars[4] = h4->pars.pcm_rightjustify;
+ skb = __hci_cmd_sync(hu->hdev, 0xfc1e, sizeof(pcm_format_pars),
+ pcm_format_pars, HCI_INIT_TIMEOUT);
+ if (IS_ERR(skb)) {
+ err = PTR_ERR(skb);
+ BT_ERR("bcm_setup pcm_ Format VSC failed (%d)",
+ err);
+ goto finalize;
+ }
+ kfree_skb(skb);
+ BT_DBG("bcm_setup pcm_ Format VSC succeeded");
+
+ skb = __hci_cmd_sync(hu->hdev, 0xfc22, sizeof(time_slot_number),
+ &time_slot_number, HCI_INIT_TIMEOUT);
+ if (IS_ERR(skb)) {
+ err = PTR_ERR(skb);
+ BT_ERR("bcm_setup SCO Time Slot VSC failed (%d)",
+ err);
+ goto finalize;
+ }
+ kfree_skb(skb);
+ BT_DBG("bcm_setup SCO Time Slot VSC succeeded");
+ }
+
+ /* Configure device's suspend/resume operation */
+ if (h4->pars.configure_sleep) {
+ /* Override the default */
+ sleep_pars[3] = (unsigned char)!h4->pars.bt_wake_active_low;
+ sleep_pars[4] = (unsigned char)!h4->pars.dev_wake_active_low;
+ skb = __hci_cmd_sync(hu->hdev, 0xfc27, sizeof(sleep_pars),
+ sleep_pars, HCI_INIT_TIMEOUT);
+ if (IS_ERR(skb)) {
+ err = PTR_ERR(skb);
+ BT_ERR("bcm_setup Sleep VSC failed (%d)", err);
+ goto finalize;
+ }
+ kfree_skb(skb);
+ BT_DBG("bcm_setup Set Sleep Parameters VSC succeeded");
+ }
+
+finalize:
+ release_firmware(fw);
+
+ err = btbcm_finalize(hu->hdev);
+
+ return err;
}

static const struct h4_recv_pkt bcm_recv_pkts[] = {
@@ -99,10 +538,16 @@ static int bcm_recv(struct hci_uart *hu, const void *data, int count)
if (!test_bit(HCI_UART_REGISTERED, &hu->flags))
return -EUNATCH;

+ BT_DBG("%s: %d bytes", __func__, count);
+
+ /* Make sure we're resumed */
+ bcm_ensure_wakeup(hu);
+
bcm->rx_skb = h4_recv_buf(hu->hdev, bcm->rx_skb, data, count,
bcm_recv_pkts, ARRAY_SIZE(bcm_recv_pkts));
if (IS_ERR(bcm->rx_skb)) {
int err = PTR_ERR(bcm->rx_skb);
+
BT_ERR("%s: Frame reassembly failed (%d)", hu->hdev->name, err);
return err;
}
@@ -114,7 +559,10 @@ static int bcm_enqueue(struct hci_uart *hu, struct sk_buff *skb)
{
struct bcm_data *bcm = hu->priv;

- BT_DBG("hu %p skb %p", hu, skb);
+ BT_DBG("%s: hu %p skb %p", __func__, hu, skb);
+
+ /* Make sure we're resumed */
+ bcm_ensure_wakeup(hu);

/* Prepend skb with frame type */
memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
@@ -130,13 +578,16 @@ static struct sk_buff *bcm_dequeue(struct hci_uart *hu)
return skb_dequeue(&bcm->txq);
}

-static const struct hci_uart_proto bcm_proto = {
+static struct hci_uart_proto bcm_proto = {
.id = HCI_UART_BCM,
.name = "BCM",
+ .init_speed = 115200,
+ .oper_speed = 4000000,
.open = bcm_open,
.close = bcm_close,
.flush = bcm_flush,
.setup = bcm_setup,
+ .set_baudrate = bcm_set_baudrate,
.recv = bcm_recv,
.enqueue = bcm_enqueue,
.dequeue = bcm_dequeue,
--
1.9.1

2015-06-03 21:01:43

by Ilya Faenson

[permalink] [raw]
Subject: [PATCH 4/5] Broadcom Bluetooth protocol UART support

Merge of the Broadcom logic into the Intel's btbcm implementation.

Signed-off-by: Ilya Faenson <[email protected]>
---
drivers/bluetooth/btbcm.c | 142 ++++++++++++++++++++++++++++++++++++----------
drivers/bluetooth/btbcm.h | 21 ++++++-
2 files changed, 132 insertions(+), 31 deletions(-)

diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index 728fce3..e1d5ad0 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -3,6 +3,7 @@
* Bluetooth support for Broadcom devices
*
* Copyright (C) 2015 Intel Corporation
+ * Copyright (C) 2015 Broadcom Corporation
*
*
* This program is free software; you can redistribute it and/or modify
@@ -32,7 +33,8 @@

#define VERSION "0.1"

-#define BDADDR_BCM20702A0 (&(bdaddr_t) {{0x00, 0xa0, 0x02, 0x70, 0x20, 0x00}})
+#define BDADDR_BCM20702A0 (&(bdaddr_t) {{0x00, 0xa0, 0x02, 0x70, 0x20, 0x00} })
+#define BDADDR_BCM4324B3 (&(bdaddr_t) {{0x00, 0x00, 0x00, 0xb3, 0x24, 0x43} })

int btbcm_check_bdaddr(struct hci_dev *hdev)
{
@@ -43,6 +45,7 @@ int btbcm_check_bdaddr(struct hci_dev *hdev)
HCI_INIT_TIMEOUT);
if (IS_ERR(skb)) {
int err = PTR_ERR(skb);
+
BT_ERR("%s: BCM: Reading device address failed (%d)",
hdev->name, err);
return err;
@@ -56,10 +59,11 @@ int btbcm_check_bdaddr(struct hci_dev *hdev)

bda = (struct hci_rp_read_bd_addr *)skb->data;

- /* The address 00:20:70:02:A0:00 indicates a BCM20702A0 controller
- * with no configured address.
+ /* Check if the address indicates a controller with no configured
+ * address.
*/
- if (!bacmp(&bda->bdaddr, BDADDR_BCM20702A0)) {
+ if (!bacmp(&bda->bdaddr, BDADDR_BCM20702A0) ||
+ !bacmp(&bda->bdaddr, BDADDR_BCM4324B3)) {
BT_INFO("%s: BCM: Using default device address (%pMR)",
hdev->name, &bda->bdaddr);
set_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks);
@@ -89,21 +93,14 @@ int btbcm_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
}
EXPORT_SYMBOL_GPL(btbcm_set_bdaddr);

-int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
+int btbcm_patchram(struct hci_dev *hdev, const struct firmware *fw)
{
const struct hci_command_hdr *cmd;
- const struct firmware *fw;
const u8 *fw_ptr;
size_t fw_size;
struct sk_buff *skb;
u16 opcode;
- int err;
-
- err = request_firmware(&fw, firmware, &hdev->dev);
- if (err < 0) {
- BT_INFO("%s: BCM: Patch %s not found", hdev->name, firmware);
- return err;
- }
+ int err = 0;

/* Start Download */
skb = __hci_cmd_sync(hdev, 0xfc2e, 0, NULL, HCI_INIT_TIMEOUT);
@@ -129,8 +126,7 @@ int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
fw_size -= sizeof(*cmd);

if (fw_size < cmd->plen) {
- BT_ERR("%s: BCM: Patch %s is corrupted", hdev->name,
- firmware);
+ BT_ERR("%s: BCM: Patch is corrupted", hdev->name);
err = -EINVAL;
goto done;
}
@@ -156,7 +152,6 @@ int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
msleep(250);

done:
- release_firmware(fw);
return err;
}
EXPORT_SYMBOL(btbcm_patchram);
@@ -168,6 +163,7 @@ static int btbcm_reset(struct hci_dev *hdev)
skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
if (IS_ERR(skb)) {
int err = PTR_ERR(skb);
+
BT_ERR("%s: BCM: Reset failed (%d)", hdev->name, err);
return err;
}
@@ -242,9 +238,101 @@ static const struct {
const char *name;
} bcm_uart_subver_table[] = {
{ 0x410e, "BCM43341B0" }, /* 002.001.014 */
+ { 0x4406, "BCM4324B3" }, /* 002.004.006 */
+ { 0x610c, "BCM4354_003.001.012.0306.0659"}, /* 003.001.012 */
{ }
};

+int btbcm_initialize(struct hci_dev *hdev, char *fw_name, size_t len)
+{
+ u16 subver, rev;
+ const char *hw_name = NULL;
+ struct sk_buff *skb;
+ struct hci_rp_read_local_version *ver;
+ int i, err;
+
+ /* Reset */
+ err = btbcm_reset(hdev);
+ if (err)
+ return err;
+
+ /* Read Local Version Info */
+ skb = btbcm_read_local_version(hdev);
+ if (IS_ERR(skb))
+ return PTR_ERR(skb);
+
+ ver = (struct hci_rp_read_local_version *)skb->data;
+ rev = le16_to_cpu(ver->hci_rev);
+ subver = le16_to_cpu(ver->lmp_subver);
+ kfree_skb(skb);
+
+ /* Read Verbose Config Version Info */
+ skb = btbcm_read_verbose_config(hdev);
+ if (IS_ERR(skb))
+ return PTR_ERR(skb);
+
+ BT_INFO("%s: BCM: chip id %u", hdev->name, skb->data[1]);
+ kfree_skb(skb);
+
+ switch ((rev & 0xf000) >> 12) {
+ case 0:
+ case 1:
+ case 3:
+ for (i = 0; bcm_uart_subver_table[i].name; i++) {
+ if (subver == bcm_uart_subver_table[i].subver) {
+ hw_name = bcm_uart_subver_table[i].name;
+ break;
+ }
+ }
+
+ snprintf(fw_name, len, "brcm/%s.hcd", hw_name ? : "BCM");
+ break;
+ default:
+ return 0;
+ }
+
+ BT_INFO("%s: %s (%3.3u.%3.3u.%3.3u) build %4.4u", hdev->name,
+ hw_name ? : "BCM", (subver & 0x7000) >> 13,
+ (subver & 0x1f00) >> 8, (subver & 0x00ff), rev & 0x0fff);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(btbcm_initialize);
+
+int btbcm_finalize(struct hci_dev *hdev)
+{
+ struct sk_buff *skb;
+ struct hci_rp_read_local_version *ver;
+ u16 subver, rev;
+ int err;
+
+ /* Reset */
+ err = btbcm_reset(hdev);
+ if (err)
+ return err;
+
+ /* Read Local Version Info */
+ skb = btbcm_read_local_version(hdev);
+ if (IS_ERR(skb))
+ return PTR_ERR(skb);
+
+ ver = (struct hci_rp_read_local_version *)skb->data;
+ rev = le16_to_cpu(ver->hci_rev);
+ subver = le16_to_cpu(ver->lmp_subver);
+ kfree_skb(skb);
+
+ BT_INFO("%s: BCM (%3.3u.%3.3u.%3.3u) build %4.4u", hdev->name,
+ (subver & 0x7000) >> 13, (subver & 0x1f00) >> 8,
+ (subver & 0x00ff), rev & 0x0fff);
+
+ btbcm_check_bdaddr(hdev);
+
+ set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(btbcm_finalize);
+
static const struct {
u16 subver;
const char *name;
@@ -265,6 +353,7 @@ static const struct {
int btbcm_setup_patchram(struct hci_dev *hdev)
{
char fw_name[64];
+ const struct firmware *fw;
u16 subver, rev, pid, vid;
const char *hw_name = NULL;
struct sk_buff *skb;
@@ -295,17 +384,6 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
kfree_skb(skb);

switch ((rev & 0xf000) >> 12) {
- case 0:
- for (i = 0; bcm_uart_subver_table[i].name; i++) {
- if (subver == bcm_uart_subver_table[i].subver) {
- hw_name = bcm_uart_subver_table[i].name;
- break;
- }
- }
-
- snprintf(fw_name, sizeof(fw_name), "brcm/%s.hcd",
- hw_name ? : "BCM");
- break;
case 1:
case 2:
/* Read USB Product Info */
@@ -335,9 +413,15 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
hw_name ? : "BCM", (subver & 0x7000) >> 13,
(subver & 0x1f00) >> 8, (subver & 0x00ff), rev & 0x0fff);

- err = btbcm_patchram(hdev, fw_name);
- if (err == -ENOENT)
+ err = request_firmware(&fw, fw_name, &hdev->dev);
+ if (err < 0) {
+ BT_INFO("%s: BCM: Patch %s not found", hdev->name, fw_name);
return 0;
+ }
+
+ btbcm_patchram(hdev, fw);
+
+ release_firmware(fw);

/* Reset */
err = btbcm_reset(hdev);
diff --git a/drivers/bluetooth/btbcm.h b/drivers/bluetooth/btbcm.h
index eb6ab5f..32599be 100644
--- a/drivers/bluetooth/btbcm.h
+++ b/drivers/bluetooth/btbcm.h
@@ -21,15 +21,20 @@
*
*/

+#include <linux/firmware.h>
+
#if IS_ENABLED(CONFIG_BT_BCM)

int btbcm_check_bdaddr(struct hci_dev *hdev);
int btbcm_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr);
-int btbcm_patchram(struct hci_dev *hdev, const char *firmware);
+int btbcm_patchram(struct hci_dev *hdev, const struct firmware *fw);

int btbcm_setup_patchram(struct hci_dev *hdev);
int btbcm_setup_apple(struct hci_dev *hdev);

+int btbcm_initialize(struct hci_dev *hdev, char *fw_name, size_t len);
+int btbcm_finalize(struct hci_dev *hdev);
+
#else

static inline int btbcm_check_bdaddr(struct hci_dev *hdev)
@@ -42,7 +47,8 @@ static inline int btbcm_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
return -EOPNOTSUPP;
}

-static inline int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
+static inline int btbcm_patchram(struct hci_dev *hdev, const struct firmware
+ *fw)
{
return -EOPNOTSUPP;
}
@@ -57,4 +63,15 @@ static inline int btbcm_setup_apple(struct hci_dev *hdev)
return 0;
}

+static inline int btbcm_initialize(struct hci_dev *hdev, char *fw_name,
+ size_t len)
+{
+ return 0;
+}
+
+static inline int btbcm_finalize(struct hci_dev *hdev)
+{
+ return 0;
+}
+
#endif
--
1.9.1


2015-06-03 21:01:42

by Ilya Faenson

[permalink] [raw]
Subject: [PATCH 3/5] Broadcom Bluetooth UART Platform Driver

Introduce the device tree enumerated Broadcom Bluetooth UART driver.

Signed-off-by: Ilya Faenson <[email protected]>
---
drivers/bluetooth/Kconfig | 9 +
drivers/bluetooth/Makefile | 1 +
drivers/bluetooth/btbcm_uart.c | 673 +++++++++++++++++++++++++++++++++++++++++
drivers/bluetooth/btbcm_uart.h | 89 ++++++
4 files changed, 772 insertions(+)
create mode 100644 drivers/bluetooth/btbcm_uart.c
create mode 100644 drivers/bluetooth/btbcm_uart.h

diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index 2e77707..5eee3ed 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -143,6 +143,7 @@ config BT_HCIUART_BCM
bool "Broadcom protocol support"
depends on BT_HCIUART
select BT_HCIUART_H4
+ select BT_UART_BCM
select BT_BCM
help
The Broadcom protocol support enables Bluetooth HCI over serial
@@ -150,6 +151,14 @@ config BT_HCIUART_BCM

Say Y here to compile support for Broadcom protocol.

+config BT_UART_BCM
+ tristate "Broadcom BT UART driver"
+ depends on BT_HCIUART_H4 && TTY
+ help
+ This driver supports the HCI_UART_BT protocol.
+
+ It manages Bluetooth UART device properties and GPIOs.
+
config BT_HCIBCM203X
tristate "HCI BCM203x USB driver"
depends on USB
diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
index f40e194..e908a88 100644
--- a/drivers/bluetooth/Makefile
+++ b/drivers/bluetooth/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_BT_MRVL) += btmrvl.o
obj-$(CONFIG_BT_MRVL_SDIO) += btmrvl_sdio.o
obj-$(CONFIG_BT_WILINK) += btwilink.o
obj-$(CONFIG_BT_BCM) += btbcm.o
+obj-$(CONFIG_BT_UART_BCM) += btbcm_uart.o
obj-$(CONFIG_BT_RTL) += btrtl.o

btmrvl-y := btmrvl_main.o
diff --git a/drivers/bluetooth/btbcm_uart.c b/drivers/bluetooth/btbcm_uart.c
new file mode 100644
index 0000000..ccd02a9
--- /dev/null
+++ b/drivers/bluetooth/btbcm_uart.c
@@ -0,0 +1,673 @@
+/*
+ * Bluetooth BCM UART Driver
+ *
+ * Copyright (c) 2015 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/module.h>
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/fcntl.h>
+#include <linux/interrupt.h>
+#include <linux/ptrace.h>
+#include <linux/poll.h>
+
+#include <linux/slab.h>
+#include <linux/tty.h>
+#include <linux/errno.h>
+#include <linux/string.h>
+#include <linux/signal.h>
+#include <linux/ioctl.h>
+#include <linux/skbuff.h>
+#include <linux/list.h>
+
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci_core.h>
+
+#include <linux/gpio/consumer.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/of_platform.h>
+
+#include "btbcm_uart.h"
+
+static int idle_timeout = 5;
+module_param(idle_timeout, int, 0);
+MODULE_PARM_DESC(idle_timeout, "Bluetooth idle timeout in seconds");
+
+/* Device context */
+struct bcm_device {
+ struct list_head list;
+
+ struct platform_device *pdev;
+ struct gpio_desc *bt_wake_gpio;
+ struct gpio_desc *dev_wake_gpio;
+ struct gpio_desc *reg_on_gpio;
+ int bt_wake_irq;
+ int dev_wake_active_low;
+ int reg_on_active_low;
+ int bt_wake_active_low;
+ bool configure_sleep;
+ u32 manual_fc;
+ u32 oper_speed;
+ bool configure_audio;
+ u32 pcm_clockmode;
+ u32 pcm_fillmethod;
+ u32 pcm_fillnum;
+ u32 pcm_fillvalue;
+ u32 pcm_incallbitclock;
+ u32 pcm_lsbfirst;
+ u32 pcm_rightjustify;
+ u32 pcm_routing;
+ u32 pcm_shortframesync;
+ u32 pcm_syncmode;
+
+ char tty_name[64];
+
+ struct btbcm_uart_callbacks protocol_callbacks;
+ struct work_struct wakeup_work;
+};
+
+/* List of BCM BT UART devices */
+static DEFINE_SPINLOCK(device_list_lock);
+static LIST_HEAD(device_list);
+
+/*
+ * Calling the BCM protocol at lower execution priority
+ */
+static void bcm_bt_wakeup_task(struct work_struct *ws)
+{
+ int gpio_value;
+ struct bcm_device *p_bcm_device =
+ container_of(ws, struct bcm_device, wakeup_work);
+
+ if (!p_bcm_device) {
+ BT_DBG("%s - failing, no device", __func__);
+ return;
+ }
+
+ /* Make sure the device is resumed */
+ gpio_value = !p_bcm_device->dev_wake_active_low;
+ if (p_bcm_device->dev_wake_gpio) {
+ gpiod_set_value(p_bcm_device->dev_wake_gpio, gpio_value);
+ BT_DBG("%s - resume %d written, delaying 15 ms",
+ __func__, gpio_value);
+ mdelay(15);
+ }
+
+ /* Let the protocol know it's time to wake up */
+ if (p_bcm_device->protocol_callbacks.p_wakeup)
+ p_bcm_device->protocol_callbacks.p_wakeup(
+ p_bcm_device->protocol_callbacks.context);
+}
+
+/*
+ * Interrupt routine for the wake from the device
+ */
+static irqreturn_t bcm_bt_uart_isr(int irq, void *context)
+{
+ unsigned int bt_wake;
+ struct bcm_device *p = (struct bcm_device *)context;
+
+ bt_wake = gpiod_get_value(p->bt_wake_gpio);
+ BT_DBG("%s with bt_wake of %d (active_low %d), req bh",
+ __func__, bt_wake, p->bt_wake_active_low);
+
+ /* Defer the actual processing to the platform work queue */
+ schedule_work(&p->wakeup_work);
+ return IRQ_HANDLED;
+}
+
+/*
+ * Device instance startup
+ */
+static int bcm_bt_uart_probe(struct platform_device *pdev)
+{
+ int ret = 0;
+ struct device_node *np = pdev->dev.of_node;
+ const char *tty_name;
+ struct bcm_device *p_bcm_device = NULL;
+
+ p_bcm_device = devm_kzalloc(&pdev->dev, sizeof(*p_bcm_device),
+ GFP_KERNEL);
+ if (!p_bcm_device) {
+ BT_DBG("%s - failing due to no memory", __func__);
+ return -ENOMEM;
+ }
+ p_bcm_device->pdev = pdev;
+ BT_DBG("%s %p context", __func__, p_bcm_device);
+
+ /* Get dev wake GPIO */
+ p_bcm_device->dev_wake_gpio = gpiod_get(&pdev->dev, "bt-wake");
+ BT_DBG("%s - gpiod_get for bt-wake returned %p",
+ __func__, p_bcm_device->dev_wake_gpio);
+ if (IS_ERR(p_bcm_device->dev_wake_gpio)) {
+ ret = PTR_ERR(p_bcm_device->dev_wake_gpio);
+ if (ret != -ENOENT) {
+ dev_err(&pdev->dev,
+ "%s - dev_wake GPIO: %d\n", __func__, ret);
+ }
+ p_bcm_device->dev_wake_gpio = NULL;
+ } else {
+ int gpio_value;
+
+ p_bcm_device->dev_wake_active_low = gpiod_is_active_low
+ (p_bcm_device->dev_wake_gpio);
+ BT_DBG("%s - dev_wake a-low is %d (cans %d)",
+ __func__, p_bcm_device->dev_wake_active_low,
+ gpiod_cansleep(p_bcm_device->dev_wake_gpio));
+
+ /* configure dev_wake as output with init resumed state */
+ gpio_value = !p_bcm_device->dev_wake_active_low;
+ ret = gpiod_direction_output(p_bcm_device->dev_wake_gpio,
+ gpio_value);
+ if (ret < 0) {
+ dev_err(&pdev->dev,
+ "%s s dev_wake GPIO: %d\n", __func__, ret);
+ gpiod_put(p_bcm_device->dev_wake_gpio);
+ p_bcm_device->dev_wake_gpio = NULL;
+ goto end;
+ } else {
+ BT_DBG("%s - dev_wake set to %d", __func__,
+ gpio_value);
+ }
+ }
+
+ /* Get power on/off GPIO */
+ p_bcm_device->reg_on_gpio = gpiod_get(&pdev->dev, "bt-reg-on");
+ BT_DBG("%s - gpiod_get for bt-reg-on returned %p", __func__,
+ p_bcm_device->reg_on_gpio);
+ if (IS_ERR(p_bcm_device->reg_on_gpio)) {
+ ret = PTR_ERR(p_bcm_device->reg_on_gpio);
+ if (ret != -ENOENT) {
+ dev_err(&pdev->dev,
+ "%s - reg_on GPIO: %d\n", __func__, ret);
+ }
+ p_bcm_device->reg_on_gpio = NULL;
+ } else {
+ int poweron_flag;
+
+ p_bcm_device->reg_on_active_low = gpiod_is_active_low
+ (p_bcm_device->reg_on_gpio);
+ BT_DBG("%s - reg_on a-low is %d (cans %d)",
+ __func__, p_bcm_device->reg_on_active_low,
+ gpiod_cansleep(p_bcm_device->reg_on_gpio));
+
+ /* configure reg_on as output with init on state */
+ poweron_flag = !p_bcm_device->reg_on_active_low;
+ ret = gpiod_direction_output(p_bcm_device->reg_on_gpio,
+ poweron_flag);
+ if (ret < 0) {
+ dev_err(&pdev->dev,
+ "%s set reg_on GPIO: %d\n", __func__, ret);
+ gpiod_put(p_bcm_device->reg_on_gpio);
+ p_bcm_device->reg_on_gpio = NULL;
+ } else {
+ BT_DBG("%s - reg_on initially set to %d", __func__,
+ poweron_flag);
+ }
+ }
+
+ platform_set_drvdata(pdev, p_bcm_device);
+ /* Must be done before interrupt is requested */
+ INIT_WORK(&p_bcm_device->wakeup_work, bcm_bt_wakeup_task);
+
+ /* Get bt host wake GPIO */
+ p_bcm_device->bt_wake_gpio = gpiod_get(&pdev->dev, "bt-host-wake");
+ BT_DBG("%s - gpiod_get for bt-host-wake returned %p", __func__,
+ p_bcm_device->bt_wake_gpio);
+ if (IS_ERR(p_bcm_device->bt_wake_gpio)) {
+ ret = PTR_ERR(p_bcm_device->bt_wake_gpio);
+ if (ret != -ENOENT) {
+ dev_err(&pdev->dev,
+ "%s - bt_wake GPIO: %d\n", __func__, ret);
+ }
+ p_bcm_device->bt_wake_gpio = NULL;
+ } else {
+ /* configure bt_wake as input */
+ ret = gpiod_direction_input(p_bcm_device->bt_wake_gpio);
+ if (ret < 0) {
+ dev_err(&pdev->dev,
+ "%s set bt_wake GPIO: %d\n", __func__, ret);
+ gpiod_put(p_bcm_device->bt_wake_gpio);
+ p_bcm_device->bt_wake_gpio = NULL;
+ } else {
+ p_bcm_device->bt_wake_active_low = gpiod_is_active_low
+ (p_bcm_device->bt_wake_gpio);
+ BT_DBG("%s -bt_wake a-low is %d(cans%d)",
+ __func__, p_bcm_device->bt_wake_active_low,
+ gpiod_cansleep(p_bcm_device->bt_wake_gpio));
+ p_bcm_device->bt_wake_irq = gpiod_to_irq
+ (p_bcm_device->bt_wake_gpio);
+ if (p_bcm_device->bt_wake_irq < 0) {
+ dev_err(&pdev->dev,
+ "%s - HOST_WAKE IRQ: %d\n", __func__, ret);
+ } else {
+ unsigned long intflags = IRQF_TRIGGER_RISING;
+
+ if (p_bcm_device->bt_wake_active_low)
+ intflags = IRQF_TRIGGER_FALLING;
+
+ ret = request_irq(p_bcm_device->bt_wake_irq,
+ bcm_bt_uart_isr,
+ intflags, "bt_host_wake",
+ p_bcm_device);
+ if (ret < 0) {
+ dev_err(&pdev->dev,
+ "%s - failed IRQ %d: %d",
+ __func__,
+ p_bcm_device->bt_wake_irq, ret);
+ } else {
+ BT_DBG("%s - IRQ %d", __func__,
+ p_bcm_device->bt_wake_irq);
+ }
+ }
+ }
+ }
+
+ p_bcm_device->configure_sleep = of_property_read_bool(
+ np, "configure-sleep");
+ BT_DBG("configure-sleep read as %d", p_bcm_device->configure_sleep);
+ p_bcm_device->manual_fc = 0;
+ if (!of_property_read_u32(np, "manual-fc",
+ &p_bcm_device->manual_fc)) {
+ BT_DBG("manual-fc read as %d",
+ p_bcm_device->manual_fc);
+ }
+ p_bcm_device->oper_speed = 3000000;
+ if (!of_property_read_u32(
+ np, "oper-speed",
+ &p_bcm_device->oper_speed)) {
+ BT_DBG("oper-speed read as %d",
+ p_bcm_device->oper_speed);
+ }
+ p_bcm_device->configure_audio = of_property_read_bool(
+ np, "configure-audio");
+ BT_DBG("configure-audio read as %d", p_bcm_device->configure_audio);
+ if (p_bcm_device->configure_audio) {
+ /* Defaults for audio */
+ p_bcm_device->pcm_clockmode = 0;
+ p_bcm_device->pcm_fillmethod = 2;
+ p_bcm_device->pcm_fillnum = 0;
+ p_bcm_device->pcm_fillvalue = 3;
+ p_bcm_device->pcm_incallbitclock = 0;
+ p_bcm_device->pcm_lsbfirst = 0;
+ p_bcm_device->pcm_rightjustify = 0;
+ p_bcm_device->pcm_routing = 0;
+ p_bcm_device->pcm_shortframesync = 0;
+ p_bcm_device->pcm_syncmode = 0;
+
+ if (!of_property_read_u32(np, "pcm-clockmode",
+ &p_bcm_device->pcm_clockmode))
+ BT_DBG("pcm-clockmode read as %d",
+ p_bcm_device->pcm_clockmode);
+ if (!of_property_read_u32(np, "pcm-fillmethod",
+ &p_bcm_device->pcm_fillmethod))
+ BT_DBG("pcm-fillmethod readas %d",
+ p_bcm_device->pcm_fillmethod);
+ if (!of_property_read_u32(np, "pcm-fillnum",
+ &p_bcm_device->pcm_fillnum))
+ BT_DBG("pcm-fillnum read as %d",
+ p_bcm_device->pcm_fillnum);
+ if (!of_property_read_u32(np, "pcm-fillvalue",
+ &p_bcm_device->pcm_fillvalue))
+ BT_DBG("pcm-fillvalue read as %d",
+ p_bcm_device->pcm_fillvalue);
+ if (!of_property_read_u32(np, "pcm-incallbitclock",
+ &p_bcm_device->pcm_incallbitclock))
+ BT_DBG("pcm-incallbitclock read as %d",
+ p_bcm_device->pcm_incallbitclock);
+ if (!of_property_read_u32(np, "pcm-lsbfirst",
+ &p_bcm_device->pcm_lsbfirst))
+ BT_DBG("pcm-lsbfirst read as %d",
+ p_bcm_device->pcm_lsbfirst);
+ if (!of_property_read_u32(np, "pcm-rightjustify",
+ &p_bcm_device->pcm_rightjustify))
+ BT_DBG("pcm-rightjustify read as %d",
+ p_bcm_device->pcm_rightjustify);
+ if (!of_property_read_u32(np, "pcm-routing",
+ &p_bcm_device->pcm_routing))
+ BT_DBG("pcm-routing read as %d",
+ p_bcm_device->pcm_routing);
+ if (!of_property_read_u32(np, "pcm-shortframesync",
+ &p_bcm_device->pcm_shortframesync))
+ BT_DBG("pcm-shortframesync read as %d",
+ p_bcm_device->pcm_shortframesync);
+ if (!of_property_read_u32(np, "pcm-syncmode",
+ &p_bcm_device->pcm_syncmode))
+ BT_DBG("pcm-syncmode read as %d",
+ p_bcm_device->pcm_syncmode);
+ }
+
+ if (!of_property_read_string(np, "tty", &tty_name)) {
+ strcpy(p_bcm_device->tty_name, tty_name);
+ BT_DBG("tty name read as %s", p_bcm_device->tty_name);
+ }
+
+ BT_DBG("idle_timeout set as %d", idle_timeout);
+
+ ret = 0; /* If we made it here, we're fine */
+
+ /* Place this instance on the device list */
+ spin_lock(&device_list_lock);
+ list_add_tail(&p_bcm_device->list, &device_list);
+ spin_unlock(&device_list_lock);
+
+end:
+ if (ret) {
+ if (p_bcm_device->reg_on_gpio) {
+ gpiod_put(p_bcm_device->reg_on_gpio);
+ p_bcm_device->reg_on_gpio = NULL;
+ }
+ if (p_bcm_device->bt_wake_gpio) {
+ gpiod_put(p_bcm_device->bt_wake_gpio);
+ p_bcm_device->bt_wake_gpio = NULL;
+ }
+ if (p_bcm_device->dev_wake_gpio) {
+ gpiod_put(p_bcm_device->dev_wake_gpio);
+ p_bcm_device->dev_wake_gpio = NULL;
+ }
+ }
+
+ BT_DBG("%s with the result %d", __func__, ret);
+ return ret;
+}
+
+/*
+ * Device instance removal
+ */
+static int bcm_bt_uart_remove(struct platform_device *pdev)
+{
+ struct bcm_device *p_bcm_device = platform_get_drvdata(pdev);
+
+ if (p_bcm_device == NULL) {
+ BT_DBG("%s - logic error, no probe?!", __func__);
+ return 0;
+ }
+
+ BT_DBG("%s %p context", __func__, p_bcm_device);
+
+ spin_lock(&device_list_lock);
+ list_del(&p_bcm_device->list);
+ spin_unlock(&device_list_lock);
+
+ BT_DBG("%s - freeing interrupt %d", __func__,
+ p_bcm_device->bt_wake_irq);
+ free_irq(p_bcm_device->bt_wake_irq, p_bcm_device);
+
+ if (p_bcm_device->reg_on_gpio) {
+ BT_DBG("%s - releasing reg_on_gpio", __func__);
+ gpiod_put(p_bcm_device->reg_on_gpio);
+ p_bcm_device->reg_on_gpio = NULL;
+ }
+
+ if (p_bcm_device->dev_wake_gpio) {
+ BT_DBG("%s - releasing dev_wake_gpio", __func__);
+ gpiod_put(p_bcm_device->dev_wake_gpio);
+ p_bcm_device->dev_wake_gpio = NULL;
+ }
+
+ if (p_bcm_device->bt_wake_gpio) {
+ BT_DBG("%s - releasing bt_wake_gpio", __func__);
+ gpiod_put(p_bcm_device->bt_wake_gpio);
+ p_bcm_device->bt_wake_gpio = NULL;
+ }
+
+ BT_DBG("%s %p done", __func__, p_bcm_device);
+ return 0;
+}
+
+/*
+ * Platform resume callback
+ */
+static int bcm_bt_uart_resume(struct device *pdev)
+{
+ int gpio_value;
+ struct bcm_device *p_bcm_device = platform_get_drvdata(
+ to_platform_device(pdev));
+
+ if (p_bcm_device == NULL) {
+ BT_DBG("%s - logic error, no device?!", __func__);
+ return 0;
+ }
+
+ BT_DBG("%s %p", __func__, p_bcm_device);
+
+ gpio_value = !p_bcm_device->dev_wake_active_low;
+ if (p_bcm_device->dev_wake_gpio) {
+ gpiod_set_value(p_bcm_device->dev_wake_gpio, gpio_value);
+ BT_DBG("%s - %d written, delaying 15 ms", __func__,
+ gpio_value);
+ mdelay(15);
+ }
+
+ /* Let the protocol know the platform is resuming */
+ if (p_bcm_device->protocol_callbacks.p_resume)
+ p_bcm_device->protocol_callbacks.p_resume(
+ p_bcm_device->protocol_callbacks.context);
+
+ return 0;
+}
+
+/*
+ * Platform suspend callback
+ */
+static int bcm_bt_uart_suspend(struct device *pdev)
+{
+ int gpio_value;
+ struct bcm_device *p_bcm_device = platform_get_drvdata(
+ to_platform_device(pdev));
+
+ if (p_bcm_device == NULL) {
+ BT_DBG("%s - logic error, no device?!", __func__);
+ return 0;
+ }
+
+ BT_DBG("%s %p", __func__, p_bcm_device);
+
+ /* Let the protocol know the platform is suspending */
+ if (p_bcm_device->protocol_callbacks.p_suspend)
+ p_bcm_device->protocol_callbacks.p_suspend(
+ p_bcm_device->protocol_callbacks.context);
+
+ /* Suspend the device */
+ if (p_bcm_device->dev_wake_gpio) {
+ gpio_value = !!p_bcm_device->dev_wake_active_low;
+ gpiod_set_value(p_bcm_device->dev_wake_gpio, gpio_value);
+ BT_DBG("%s - %d written, delaying 15 ms", __func__,
+ gpio_value);
+ mdelay(15);
+ }
+
+ return 0;
+}
+
+/*
+ * Entry point for calls from the protocol
+ */
+int btbcm_uart_control(int action, void *device_context,
+ void *p_data, unsigned long *p_size)
+{
+ struct btbcm_uart_callbacks *pc;
+ struct btbcm_uart_parameters *pp = p_data; /* for pars action only */
+ int ret = 0;
+ int gpio_value, poweron_flag;
+ struct bcm_device *p_bcm_device = device_context;
+ struct list_head *ptr;
+ bool is_found = false;
+
+ /* Special processing for the callback configuration */
+ if (action == BTBCM_UART_ACTION_CONFIGURE_CALLBACKS) {
+ pc = p_data;
+
+ BT_DBG("%s - configure callbacks", __func__);
+ if (p_data == NULL || *p_size != sizeof(struct
+ btbcm_uart_callbacks) || (pc->interface_version !=
+ BTBCM_UART_INTERFACE_VERSION)) {
+ BT_DBG("%s - callbacks mismatch!", __func__);
+ return -E2BIG;
+ }
+
+ BT_DBG("%s - configure callbacks for %s(%p)", __func__,
+ pc->name, pc->context);
+ if (p_bcm_device == NULL) {
+ spin_lock(&device_list_lock);
+ list_for_each(ptr, &device_list) {
+ p_bcm_device = list_entry(ptr, struct
+ bcm_device, list);
+ if (!strcmp(p_bcm_device->tty_name, pc->name)) {
+ is_found = true;
+ break;
+ }
+ }
+
+ spin_unlock(&device_list_lock);
+ if (!is_found) {
+ BT_DBG("%s - no device!", __func__);
+ return -ENOENT;
+ }
+ }
+
+ p_bcm_device->protocol_callbacks = *pc;
+ memcpy(p_data, &p_bcm_device, sizeof(p_bcm_device));
+ *p_size = sizeof(p_bcm_device);
+ return ret;
+ }
+
+ /* All other requests must have the right context */
+ if (p_bcm_device == NULL) {
+ BT_DBG("%s - failing, no device", __func__);
+ return -ENOENT;
+ }
+
+ switch (action) {
+ case BTBCM_UART_ACTION_POWER_ON:
+ BT_DBG("%s %p - power on", __func__, device_context);
+ if (p_bcm_device->reg_on_gpio) {
+ poweron_flag = !p_bcm_device->reg_on_active_low;
+ gpiod_set_value(p_bcm_device->reg_on_gpio,
+ poweron_flag);
+ BT_DBG("%s - pwron %d, delay 15 ms", __func__,
+ poweron_flag);
+ mdelay(15);
+ }
+ break;
+
+ case BTBCM_UART_ACTION_POWER_OFF:
+ BT_DBG("%s %p - power off", __func__, device_context);
+ if (p_bcm_device->reg_on_gpio) {
+ poweron_flag = p_bcm_device->reg_on_active_low;
+ gpiod_set_value(p_bcm_device->reg_on_gpio,
+ poweron_flag);
+ BT_DBG("%s - pwroff %d, delay 15 ms", __func__,
+ poweron_flag);
+ mdelay(15);
+ }
+ break;
+
+ case BTBCM_UART_ACTION_RESUME:
+ BT_DBG("%s %p - resume", __func__, device_context);
+ if (p_bcm_device->dev_wake_gpio) {
+ gpio_value = !p_bcm_device->dev_wake_active_low;
+ gpiod_set_value(p_bcm_device->dev_wake_gpio,
+ gpio_value);
+ BT_DBG("%s - resume %d, delay 15 ms", __func__,
+ gpio_value);
+ mdelay(15);
+ }
+ break;
+
+ case BTBCM_UART_ACTION_SUSPEND:
+ BT_DBG("%s %p - suspend", __func__, device_context);
+ if (p_bcm_device->dev_wake_gpio) {
+ gpio_value = !!p_bcm_device->dev_wake_active_low;
+ gpiod_set_value(p_bcm_device->dev_wake_gpio,
+ gpio_value);
+ BT_DBG("btbcm_uart_control - suspend %d, delay 15ms",
+ gpio_value);
+ mdelay(15);
+ }
+ break;
+
+ case BTBCM_UART_ACTION_GET_PARAMETERS:
+ BT_DBG("%s %p - get pars", __func__, device_context);
+ if ((p_data == NULL) ||
+ (*p_size < sizeof(struct btbcm_uart_parameters))) {
+ BT_DBG("%s - failing, wrong par size", __func__);
+ return -E2BIG;
+ }
+
+ memset(pp, 0, sizeof(struct btbcm_uart_parameters));
+ pp->interface_version = BTBCM_UART_INTERFACE_VERSION;
+ pp->configure_sleep = p_bcm_device->configure_sleep;
+ pp->manual_fc = p_bcm_device->manual_fc;
+ pp->dev_wake_active_low = p_bcm_device->dev_wake_active_low;
+ pp->bt_wake_active_low = p_bcm_device->bt_wake_active_low;
+ pp->idle_timeout_in_secs = idle_timeout;
+ pp->oper_speed = p_bcm_device->oper_speed;
+ pp->configure_audio = p_bcm_device->configure_audio;
+ pp->pcm_clockmode = p_bcm_device->pcm_clockmode;
+ pp->pcm_fillmethod = p_bcm_device->pcm_fillmethod;
+ pp->pcm_fillnum = p_bcm_device->pcm_fillnum;
+ pp->pcm_fillvalue = p_bcm_device->pcm_fillvalue;
+ pp->pcm_incallbitclock = p_bcm_device->pcm_incallbitclock;
+ pp->pcm_lsbfirst = p_bcm_device->pcm_lsbfirst;
+ pp->pcm_rightjustify = p_bcm_device->pcm_rightjustify;
+ pp->pcm_routing = p_bcm_device->pcm_routing;
+ pp->pcm_shortframesync = p_bcm_device->pcm_shortframesync;
+ pp->pcm_syncmode = p_bcm_device->pcm_syncmode;
+ *p_size = sizeof(struct btbcm_uart_parameters);
+ break;
+
+ default:
+ BT_DBG("%s %p unknown act %d", __func__,
+ device_context, action);
+ ret = -EINVAL;
+ break;
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL(btbcm_uart_control);
+
+/* Platform susp and resume callbacks */
+static SIMPLE_DEV_PM_OPS(bcm_bt_uart_pm_ops,
+ bcm_bt_uart_suspend, bcm_bt_uart_resume);
+
+/* Driver match table */
+static const struct of_device_id bcm_bt_uart_table[] = {
+ { .compatible = "brcm,brcm-bt-uart" },
+ {}
+};
+
+/* Driver configuration */
+static struct platform_driver bcm_bt_uart_driver = {
+ .probe = bcm_bt_uart_probe,
+ .remove = bcm_bt_uart_remove,
+ .driver = {
+ .name = "brcm_bt_uart",
+ .of_match_table = of_match_ptr(bcm_bt_uart_table),
+ .owner = THIS_MODULE,
+ .pm = &bcm_bt_uart_pm_ops,
+ },
+};
+
+module_platform_driver(bcm_bt_uart_driver);
+
+MODULE_AUTHOR("Ilya Faenson");
+MODULE_DESCRIPTION("Broadcom Bluetooth UART Driver");
+MODULE_LICENSE("GPL");
+
diff --git a/drivers/bluetooth/btbcm_uart.h b/drivers/bluetooth/btbcm_uart.h
new file mode 100644
index 0000000..fbc7285
--- /dev/null
+++ b/drivers/bluetooth/btbcm_uart.h
@@ -0,0 +1,89 @@
+/*
+ * Bluetooth BCM UART Driver Header
+ *
+ * Copyright (c) 2015 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef BTBCM_UART_H
+#define BTBCM_UART_H
+
+/* Change the version if you change anything in this header */
+#define BTBCM_UART_INTERFACE_VERSION 1
+/* Callbacks from the driver into the protocol */
+typedef void (*p_suspend_callback)(void *context);
+typedef void (*p_resume_callback)(void *context);
+typedef void (*p_wakeup_callback)(void *context);
+struct btbcm_uart_callbacks {
+ int interface_version; /* interface # compiled against */
+ void *context; /* protocol instance context */
+ char name[64]; /* protocol tty device, for example, ttyS0 */
+
+ /* client callbacks */
+ p_suspend_callback p_suspend;
+ p_resume_callback p_resume;
+ p_wakeup_callback p_wakeup;
+};
+
+/* Driver parameters retrieved from the DT or ACPI */
+struct btbcm_uart_parameters {
+ int interface_version; /* interface # compiled against */
+
+ /* Parameters themselves */
+ bool configure_sleep;
+ int manual_fc;
+ int dev_wake_active_low;
+ int bt_wake_active_low;
+ int idle_timeout_in_secs;
+ int oper_speed;
+ bool configure_audio;
+ int pcm_clockmode;
+ int pcm_fillmethod;
+ int pcm_fillnum;
+ int pcm_fillvalue;
+ int pcm_incallbitclock;
+ int pcm_lsbfirst;
+ int pcm_rightjustify;
+ int pcm_routing;
+ int pcm_shortframesync;
+ int pcm_syncmode;
+};
+
+/*
+ * Actions on the BTBCM_UART driver
+ */
+
+/* Configure protocol callbacks */
+#define BTBCM_UART_ACTION_CONFIGURE_CALLBACKS 0
+
+/* Retrieve BT device parameters */
+#define BTBCM_UART_ACTION_GET_PARAMETERS 1
+
+/* Resume the BT device via GPIO */
+#define BTBCM_UART_ACTION_RESUME 2
+
+/* Suspend the BT device via GPIO */
+#define BTBCM_UART_ACTION_SUSPEND 3
+
+/* Power the BT device off via GPIO */
+#define BTBCM_UART_ACTION_POWER_OFF 4
+
+/* Power the BT device on via GPIO */
+#define BTBCM_UART_ACTION_POWER_ON 5
+
+/* Execute an action on the BT device */
+extern int btbcm_uart_control(int action, void *device_context,
+ void *p_data, unsigned long *p_size);
+
+#endif
+
--
1.9.1

2015-06-03 21:01:41

by Ilya Faenson

[permalink] [raw]
Subject: [PATCH 2/5] Intel based H4 line discipline enhancements

This is largely enhancements implemented by Frederic Danis of Intel.
I've also added the ability to flow control the UART and improved the
UART baud rate setting some.

Signed-off-by: Ilya Faenson <[email protected]>
---
drivers/bluetooth/hci_ldisc.c | 110 ++++++++++++++++++++++++++++++++++++++++--
drivers/bluetooth/hci_uart.h | 6 +++
2 files changed, 113 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 5c9a73f..58dcb24 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -1,4 +1,4 @@
-/*
+/*_
*
* Bluetooth HCI UART driver
*
@@ -256,7 +256,8 @@ static int hci_uart_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
if (!test_bit(HCI_RUNNING, &hdev->flags))
return -EBUSY;

- BT_DBG("%s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type, skb->len);
+ BT_DBG("%s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type,
+ skb->len);

hu->proto->enqueue(hu, skb);

@@ -265,11 +266,114 @@ static int hci_uart_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
return 0;
}

+void hci_uart_flow_control_device(struct hci_uart *hu)
+{
+ struct tty_struct *tty = hu->tty;
+ struct ktermios ktermios;
+ int status;
+ unsigned int set = 0;
+ unsigned int clear = 0;
+
+ /* Disable hardware flow control */
+ ktermios = tty->termios;
+ ktermios.c_cflag &= ~CRTSCTS;
+ status = tty_set_termios(tty, &ktermios);
+ if (status)
+ BT_DBG("%s dis fc failure %d", __func__, status);
+ else
+ BT_DBG("%s hw fc disabled", __func__);
+
+ /* Clear RTS to prevent the device from sending */
+ /* (most PCs need OUT2 to enable interrupts) */
+ status = tty->driver->ops->tiocmget(tty);
+ BT_DBG("%s cur tiocm 0x%x", __func__, status);
+ set &= ~(TIOCM_OUT2 | TIOCM_RTS);
+ clear = ~set;
+ set &= TIOCM_DTR | TIOCM_RTS | TIOCM_OUT1 |
+ TIOCM_OUT2 | TIOCM_LOOP;
+ clear &= TIOCM_DTR | TIOCM_RTS | TIOCM_OUT1 |
+ TIOCM_OUT2 | TIOCM_LOOP;
+ status = tty->driver->ops->tiocmset(tty, set, clear);
+ if (status)
+ BT_DBG("%s clr RTS fail %d", __func__, status);
+ else
+ BT_DBG("%s RTS cleared", __func__);
+ status = tty->driver->ops->tiocmget(tty);
+}
+
+void hci_uart_unflow_control_device(struct hci_uart *hu)
+{
+ struct tty_struct *tty = hu->tty;
+ struct ktermios ktermios;
+ int status;
+ unsigned int set = 0;
+ unsigned int clear = 0;
+
+ status = tty->driver->ops->tiocmget(tty);
+ BT_DBG("%s cur tiocm 0x%x", __func__, status);
+ set |= (TIOCM_OUT2 | TIOCM_RTS);
+ clear = ~set;
+ set &= TIOCM_DTR | TIOCM_RTS | TIOCM_OUT1 |
+ TIOCM_OUT2 | TIOCM_LOOP;
+ clear &= TIOCM_DTR | TIOCM_RTS | TIOCM_OUT1 |
+ TIOCM_OUT2 | TIOCM_LOOP;
+ status = tty->driver->ops->tiocmset(tty, set, clear);
+ if (status)
+ BT_DBG("%s set RTS fail %d", __func__, status);
+ else
+ BT_DBG("%s RTS set", __func__);
+
+ /* Re-enable hardware flow control */
+ ktermios = tty->termios;
+ ktermios.c_cflag |= CRTSCTS;
+ status = tty_set_termios(tty, &ktermios);
+ if (status)
+ BT_DBG("%s enable fc fail %d", __func__, status);
+ else
+ BT_DBG("%s hw fc re-enabled", __func__);
+}
+
+void hci_uart_set_baudrate(struct hci_uart *hu, unsigned int speed)
+{
+ struct tty_struct *tty = hu->tty;
+ struct ktermios ktermios;
+
+ /* Bring the UART into a known state with a given baud rate */
+ ktermios = tty->termios;
+ ktermios.c_cflag &= ~CBAUD;
+ ktermios.c_iflag &= ~(IGNBRK | BRKINT | PARMRK | ISTRIP
+ | INLCR | IGNCR | ICRNL | IXON);
+ ktermios.c_oflag &= ~OPOST;
+ ktermios.c_lflag &= ~(ECHO | ECHONL | ICANON | ISIG | IEXTEN);
+ ktermios.c_cflag &= ~(CSIZE | PARENB | CBAUD);
+ ktermios.c_cflag |= CS8;
+ ktermios.c_cflag |= CRTSCTS;
+ /* ktermios.c_cflag |= BOTHER; */
+ tty_termios_encode_baud_rate(&ktermios, speed, speed);
+
+ /* tty_set_termios() return not checked as it is always 0 */
+ tty_set_termios(tty, &ktermios);
+
+ BT_DBG("%s: New tty speeds: %d/%d, cflag: 0x%x", hu->hdev->name,
+ tty->termios.c_ispeed, tty->termios.c_ospeed,
+ tty->termios.c_cflag);
+}
+
static int hci_uart_setup(struct hci_dev *hdev)
{
struct hci_uart *hu = hci_get_drvdata(hdev);
struct hci_rp_read_local_version *ver;
struct sk_buff *skb;
+ int err;
+
+ if (hu->proto->init_speed)
+ hci_uart_set_baudrate(hu, hu->proto->init_speed);
+
+ if (hu->proto->set_baudrate && hu->proto->oper_speed) {
+ err = hu->proto->set_baudrate(hu, hu->proto->oper_speed);
+ if (!err)
+ hci_uart_set_baudrate(hu, hu->proto->oper_speed);
+ }

if (hu->proto->setup)
return hu->proto->setup(hu);
@@ -647,7 +751,7 @@ static int __init hci_uart_init(void)

/* Register the tty discipline */

- memset(&hci_uart_ldisc, 0, sizeof (hci_uart_ldisc));
+ memset(&hci_uart_ldisc, 0, sizeof(hci_uart_ldisc));
hci_uart_ldisc.magic = TTY_LDISC_MAGIC;
hci_uart_ldisc.name = "n_hci";
hci_uart_ldisc.open = hci_uart_tty_open;
diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
index 72120a5..2271cc0 100644
--- a/drivers/bluetooth/hci_uart.h
+++ b/drivers/bluetooth/hci_uart.h
@@ -58,10 +58,13 @@ struct hci_uart;
struct hci_uart_proto {
unsigned int id;
const char *name;
+ unsigned int init_speed;
+ unsigned int oper_speed;
int (*open)(struct hci_uart *hu);
int (*close)(struct hci_uart *hu);
int (*flush)(struct hci_uart *hu);
int (*setup)(struct hci_uart *hu);
+ int (*set_baudrate)(struct hci_uart *hu, unsigned int speed);
int (*recv)(struct hci_uart *hu, const void *data, int len);
int (*enqueue)(struct hci_uart *hu, struct sk_buff *skb);
struct sk_buff *(*dequeue)(struct hci_uart *hu);
@@ -96,6 +99,9 @@ int hci_uart_register_proto(const struct hci_uart_proto *p);
int hci_uart_unregister_proto(const struct hci_uart_proto *p);
int hci_uart_tx_wakeup(struct hci_uart *hu);
int hci_uart_init_ready(struct hci_uart *hu);
+void hci_uart_set_baudrate(struct hci_uart *hu, unsigned int speed);
+void hci_uart_flow_control_device(struct hci_uart *hu);
+void hci_uart_unflow_control_device(struct hci_uart *hu);

#ifdef CONFIG_BT_HCIUART_H4
int h4_init(void);
--
1.9.1

2015-06-03 21:01:40

by Ilya Faenson

[permalink] [raw]
Subject: [PATCH 1/5] Broadcom Bluetooth UART Device Tree bindings

Device Tree bindings to configure the Broadcom Bluetooth UART device.
Updated not to use CamelCase and to use Intel style "oper-speed".

Signed-off-by: Ilya Faenson <[email protected]>
---
.../devicetree/bindings/net/bluetooth/btbcm.txt | 82 ++++++++++++++++++++++
1 file changed, 82 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/bluetooth/btbcm.txt

diff --git a/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
new file mode 100644
index 0000000..2679819
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
@@ -0,0 +1,82 @@
+btbcm
+------
+
+Required properties:
+
+ - compatible : must be "brcm,brcm-bt-uart".
+ - tty : tty device connected to this Bluetooth device.
+
+Optional properties:
+
+ - bt-host-wake-gpios : bt-host-wake input GPIO to be used as an interrupt.
+
+ - bt-wake-gpios : bt-wake output GPIO to be used to suspend / resume device.
+
+ - bt-reg-on-gpios : reg-on output GPIO to be used to power device on/off.
+
+ - oper-speed : Bluetooth device operational baud rate.
+ Default: 3000000.
+
+ - manual-fc : flow control UART in suspend / resume scenarios.
+ Default: 0.
+
+ - configure-sleep : configure suspend / resume flag.
+ Default: false.
+
+ - configure-audio : configure platform PCM SCO flag.
+ Default: false.
+
+ - pcm-clockmode : PCM clock mode. 0-slave, 1-master.
+ Default: 0.
+
+ - pcm-fillmethod : PCM fill method. 0 to 3.
+ Default: 2.
+
+ - pcm-fillnum : PCM number of fill bits. 0 to 3.
+ Default: 0.
+
+ - pcm-fillvalue : PCM fill value. 0 to 7.
+ Default: 3.
+
+ - pcm-incallbitclock : PCM interface rate. 0-128Kbps, 1-256Kbps, 2-512Kbps,
+ 3-1024Kbps, 4-2048Kbps.
+ Default: 0.
+
+ - pcm-lsbfirst : PCM LSB first. 0 or 1.
+ Default: 0.
+
+ - pcm-rightjustify : PCM Justify. 0-left, 1-right.
+ Default: 0.
+
+ - pcm-routing : PCM routing. 0-PCM, 1-SCO over HCI.
+ Default: 0.
+
+ - pcm-shortframesync : PCM sync. 0-short, 1-long.
+ Default: 0.
+
+ - pcmsyncmode : PCM sync mode. 0-slave, 1-master.
+ Default: 0.
+
+
+Example:
+
+ brcm4354_bt_uart: brcm4354-bt-uart {
+ compatible = "brcm,brcm-bt-uart";
+ bt-wake-gpios = <&gpio4 30 GPIO_ACTIVE_HIGH>;
+ bt-host-wake-gpios = <&gpio4 31 GPIO_ACTIVE_HIGH>;
+ tty = "ttyS0";
+ oper-speed = <3000000>;
+ configure-sleep;
+ configure-audio;
+ pcm-clockmode = <0>;
+ pcm-fillmethod = <2>;
+ pcm-fillnum = <0>;
+ pcm-fillvalue = <3>;
+ pcm-incallbitclock = <0>;
+ pcm-lsbfirst = <0>;
+ pcm-rightjustify = <0>;
+ pcm-routing = <0>;
+ pcm-shortframesync = <0>;
+ pcm-syncmode = <0>;
+ };
+
--
1.9.1