Re: [PATCH 10/10] Bluetooth: add nokia driver

From	Sebastian Reichel <sre@kernel.org>
Newsgroups	linux.kernel
Subject	Re: [PATCH 10/10] Bluetooth: add nokia driver
Date	2017-03-07 22:50 +0100
Message-ID	<tiv9v-5b2-11@gated-at.bofh.it> (permalink)
References	<thgFz-sD-7@gated-at.bofh.it> <thgFz-sD-15@gated-at.bofh.it> <tiqtd-1LM-23@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

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@kernel.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@kernel.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

Thread

[PATCH 00/10] Nokia H4+ support Sebastian Reichel <sre@kernel.org> - 2017-03-04 13:10 +0100
  [PATCH 09/10] serdev: add serdev_device_set_rts Sebastian Reichel <sre@kernel.org> - 2017-03-04 13:10 +0100
    Re: [PATCH 09/10] serdev: add serdev_device_set_rts Rob Herring <robh+dt@kernel.org> - 2017-03-07 17:10 +0100
      Re: [PATCH 09/10] serdev: add serdev_device_set_rts Sebastian Reichel <sre@kernel.org> - 2017-03-07 22:20 +0100
  [PATCH 10/10] Bluetooth: add nokia driver Sebastian Reichel <sre@kernel.org> - 2017-03-04 13:10 +0100
    Re: [PATCH 10/10] Bluetooth: add nokia driver Rob Herring <robh+dt@kernel.org> - 2017-03-07 17:50 +0100
      Re: [PATCH 10/10] Bluetooth: add nokia driver Sebastian Reichel <sre@kernel.org> - 2017-03-07 22:50 +0100
        Re: [PATCH 10/10] Bluetooth: add nokia driver Marcel Holtmann <marcel@holtmann.org> - 2017-03-07 23:00 +0100
          Re: [PATCH 10/10] Bluetooth: add nokia driver Sebastian Reichel <sre@kernel.org> - 2017-03-08 00:10 +0100
        Re: [PATCH 10/10] Bluetooth: add nokia driver Rob Herring <robh+dt@kernel.org> - 2017-03-08 15:40 +0100
  [PATCH 03/10] Bluetooth: hci_uart: add support for word alignment Sebastian Reichel <sre@kernel.org> - 2017-03-04 13:10 +0100
  [PATCH 04/10] Bluetooth: hci_uart: add serdev driver support library Sebastian Reichel <sre@kernel.org> - 2017-03-04 13:10 +0100
  [PATCH 05/10] Bluetooth: hci_serdev: do not open device in hci open Sebastian Reichel <sre@kernel.org> - 2017-03-04 13:10 +0100
  [PATCH 07/10] serdev: add serdev_device_wait_until_sent Sebastian Reichel <sre@kernel.org> - 2017-03-04 13:10 +0100
    Re: [PATCH 07/10] serdev: add serdev_device_wait_until_sent Rob Herring <robh+dt@kernel.org> - 2017-03-07 16:50 +0100
      Re: [PATCH 07/10] serdev: add serdev_device_wait_until_sent Rob Herring <robh+dt@kernel.org> - 2017-03-07 18:00 +0100
      Re: [PATCH 07/10] serdev: add serdev_device_wait_until_sent Sebastian Reichel <sre@kernel.org> - 2017-03-07 18:20 +0100

csiph-web