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]

Message-ID: <20170307210837.w5fdwc2sy3xmzci7@earth>
Date:   Tue, 7 Mar 2017 22:08:37 +0100
From:   Sebastian Reichel <sre@...nel.org>
To:     Rob Herring <robh+dt@...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

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"

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.

> > + - bluetooth-wakeup-gpios: GPIO specifier, used to wakeup the BT module
> > + - host-wakeup-gpios: GPIO specifier, used to wakeup the host processor
> 
> I think most BCM devices have these. These apply to the TI device too?
> If not, then add brcm prefix.

Yes, Nokia N950 uses a TI WL1271 and also has them. Actually the example
from below is a TI chip.

> Also need to specify the active state.

ok.

> > + - clock-names: should be "sysclk"
> > + - clocks: should contain a clock specifier for every name in clock-names
> > +
> > +Optional properties:
> > +
> > + - None
> > +
> > +Example:
> > +
> > +/ {
> > +       /* controlled (enabled/disabled) directly by BT module */
> > +       bluetooth_clk: vctcxo {
> > +               compatible = "fixed-clock";
> > +               #clock-cells = <0>;
> > +               clock-frequency = <38400000>;
> > +       };
> > +};
> > +
> > +&uart2 {
> > +       pinctrl-names = "default";
> > +       pinctrl-0 = <&uart2_pins>;
> > +
> > +       bluetooth {
> > +               compatible = "nokia,ti,wl1271-bluetooth";
> > +
> > +               reset-gpios = <&gpio1 26 GPIO_ACTIVE_LOW>; /* gpio26 */
> > +               host-wakeup-gpios = <&gpio4 5 GPIO_ACTIVE_HIGH>; /* gpio101 */
> > +               bluetooth-wakeup-gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>; /* gpio37 */
> > +
> > +               clocks = <&bluetooth_clk>;
> > +               clock-names = "sysclk";
> > +       };
> > +};

-- Sebastian

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)