linux-kernel - Re: [PATCH 10/10] Bluetooth: add nokia driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 8 Mar 2017 08:26:33 -0600
From:   Rob Herring <robh+dt@...nel.org>
To:     Sebastian Reichel <sre@...nel.org>
Cc:     Marcel Holtmann <marcel@...tmann.org>,
        Gustavo Padovan <gustavo@...ovan.org>,
        Johan Hedberg <johan.hedberg@...il.com>,
        Tony Lindgren <tony@...mide.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jslaby@...e.com>,
        Mark Rutland <mark.rutland@....com>,
        "open list:BLUETOOTH DRIVERS" <linux-bluetooth@...r.kernel.org>,
        "linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 10/10] Bluetooth: add nokia driver

On Tue, Mar 7, 2017 at 3:08 PM, Sebastian Reichel <sre@...nel.org> wrote:
> Hi Rob,
>
> On Tue, Mar 07, 2017 at 10:30:51AM -0600, Rob Herring wrote:
>> On Sat, Mar 4, 2017 at 5:58 AM, Sebastian Reichel <sre@...nel.org> wrote:
>> > This adds a driver for the Nokia H4+ protocol, which is used
>> > at least on the Nokia N9, N900 & N950.
>> >
>> > Signed-off-by: Sebastian Reichel <sre@...nel.org>
>> > ---
>> >  .../devicetree/bindings/net/nokia-bluetooth.txt    |  51 ++
>>
>> This should be separate and before the dts files.
>>
>> >  drivers/bluetooth/Kconfig                          |  12 +
>> >  drivers/bluetooth/Makefile                         |   2 +
>> >  drivers/bluetooth/hci_nokia.c                      | 839 +++++++++++++++++++++
>> >  4 files changed, 904 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/net/nokia-bluetooth.txt
>> >  create mode 100644 drivers/bluetooth/hci_nokia.c
>> >
>> > diff --git a/Documentation/devicetree/bindings/net/nokia-bluetooth.txt b/Documentation/devicetree/bindings/net/nokia-bluetooth.txt
>> > new file mode 100644
>> > index 000000000000..6c80a92f31e2
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/net/nokia-bluetooth.txt
>> > @@ -0,0 +1,51 @@
>> > +Nokia Bluetooth Chips
>> > +---------------------
>> > +
>> > +Nokia phones often come with UART connected bluetooth chips from different
>> > +vendors and modified device API. Those devices speak a protocol named H4+
>> > +by Nokia, which is similar to the H4 protocol from the Bluetooth standard.
>> > +In addition to the H4 protocol it specifies two more UART status lines for
>> > +wakeup of UART transceivers to improve power management and a few new packet
>> > +types used to negotiate uart speed.
>> > +
>> > +Required properties:
>> > +
>> > + - compatible: should be one of the following:
>> > +   * "nokia,brcm,bcm2048"
>> > +   * "nokia,ti,wl1271-bluetooth"
>>
>> Drop the chip vendors' prefix here. I don't really want to start a
>> pattern of 2 vendor prefixes.
>
> Right, I think we discussed this before, but I don't remember the
> result. How about
>
> - compatible: should contain "nokia,h4p-bluetooth" as well as one of the following:
>  * "brcm,bcm2048-nokia"
>  * "ti,wl1271-blueooth-nokia"

Sure, that's fine (other than the typo).

> For the driver it should be enough to know "nokia,h4p-bluetooth"
> actually. The device identifies itself in the negotiation reply.
>
>> > + - reset-gpios: GPIO specifier, used to reset the BT module
>>
>> Need to state active state.
>
> ok. Any suggestion about the wording? The BT chips use usually
> active low reset pin. The driver handles all GPIOs as active
> high with the DT binding translating this transparently.

Just having "active low" in there is enough.

Personally I find the GPIO translating confusing...

Rob