[PATCH 00/10] Nokia H4+ support

[PATCH 00/10] Nokia H4+ support

[Sebastian Reichel]

Hi,

As the title suggest here is another try to get the Nokia bluetooth
supported mainline. The driver is based on the serdev work from Rob
Herring, which got merged into 4.11-rc1 (this patchset is based on
recent master branch from torvalds).

The driver was tested on N950 (I can connect to different devices
with bluetoothctl and fetch their info). It does not yet run completly
stable (especially on N900), but its better than no device support
at all.

-- Sebastian

Rob Herring (1):
  Bluetooth: hci_uart: add serdev driver support library

Sebastian Reichel (9):
  ARM: dts: N9/N950: add bluetooth
  ARM: dts: N900: Add bluetooth
  Bluetooth: hci_uart: add support for word alignment
  Bluetooth: hci_serdev: do not open device in hci open
  tty: serial: omap: add UPF_BOOT_AUTOCONF flag for DT init
  serdev: add serdev_device_wait_until_sent
  serdev: add serdev_device_get_cts
  serdev: add serdev_device_set_rts
  Bluetooth: add nokia driver

 .../devicetree/bindings/net/nokia-bluetooth.txt    |  51 ++
 arch/arm/boot/dts/omap3-n900.dts                   |  23 +-
 arch/arm/boot/dts/omap3-n950-n9.dtsi               |  32 +
 drivers/bluetooth/Kconfig                          |  12 +
 drivers/bluetooth/Makefile                         |   3 +
 drivers/bluetooth/hci_h4.c                         |  17 +
 drivers/bluetooth/hci_ldisc.c                      |   4 +
 drivers/bluetooth/hci_nokia.c                      | 839 +++++++++++++++++++++
 drivers/bluetooth/hci_serdev.c                     | 365 +++++++++
 drivers/bluetooth/hci_uart.h                       |   7 +
 drivers/tty/serdev/core.c                          |  33 +
 drivers/tty/serdev/serdev-ttyport.c                |  62 +-
 drivers/tty/serial/omap-serial.c                   |   3 +
 include/linux/serdev.h                             |  12 +
 14 files changed, 1460 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/nokia-bluetooth.txt
 create mode 100644 drivers/bluetooth/hci_nokia.c
 create mode 100644 drivers/bluetooth/hci_serdev.c

-- 
2.11.0

[PATCH 00/10] Nokia H4+ support

[Sebastian Reichel]

Hi,

As the title suggest here is another try to get the Nokia bluetooth
supported mainline. The driver is based on the serdev work from Rob
Herring, which got merged into 4.11-rc1 (this patchset is based on
recent master branch from torvalds).

The driver was tested on N950 (I can connect to different devices
with bluetoothctl and fetch their info). It does not yet run completly
stable (especially on N900), but its better than no device support
at all.

-- Sebastian

Rob Herring (1):
  Bluetooth: hci_uart: add serdev driver support library

Sebastian Reichel (9):
  ARM: dts: N9/N950: add bluetooth
  ARM: dts: N900: Add bluetooth
  Bluetooth: hci_uart: add support for word alignment
  Bluetooth: hci_serdev: do not open device in hci open
  tty: serial: omap: add UPF_BOOT_AUTOCONF flag for DT init
  serdev: add serdev_device_wait_until_sent
  serdev: add serdev_device_get_cts
  serdev: add serdev_device_set_rts
  Bluetooth: add nokia driver

 .../devicetree/bindings/net/nokia-bluetooth.txt    |  51 ++
 arch/arm/boot/dts/omap3-n900.dts                   |  23 +-
 arch/arm/boot/dts/omap3-n950-n9.dtsi               |  32 +
 drivers/bluetooth/Kconfig                          |  12 +
 drivers/bluetooth/Makefile                         |   3 +
 drivers/bluetooth/hci_h4.c                         |  17 +
 drivers/bluetooth/hci_ldisc.c                      |   4 +
 drivers/bluetooth/hci_nokia.c                      | 839 +++++++++++++++++++++
 drivers/bluetooth/hci_serdev.c                     | 365 +++++++++
 drivers/bluetooth/hci_uart.h                       |   7 +
 drivers/tty/serdev/core.c                          |  33 +
 drivers/tty/serdev/serdev-ttyport.c                |  62 +-
 drivers/tty/serial/omap-serial.c                   |   3 +
 include/linux/serdev.h                             |  12 +
 14 files changed, 1460 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/nokia-bluetooth.txt
 create mode 100644 drivers/bluetooth/hci_nokia.c
 create mode 100644 drivers/bluetooth/hci_serdev.c

-- 
2.11.0

[PATCH 01/10] ARM: dts: N9/N950: add bluetooth

[Sebastian Reichel]

The Nokia N950 and N9 have a wl1271 (with nokia bootloader) bluetooth
module connected to second UART.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 arch/arm/boot/dts/omap3-n950-n9.dtsi | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi
index 5d8c4b4a4205..704f11c83165 100644
--- a/arch/arm/boot/dts/omap3-n950-n9.dtsi
+++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
@@ -58,6 +58,13 @@
 			pinctrl-0 = <&debug_leds>;
 		};
 	};
+
+	/* controlled (enabled/disabled) directly by wl1271 */
+	vctcxo: vctcxo {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <38400000>;
+	};
 };
 
 &omap3_pmx_core {
@@ -125,6 +132,15 @@
 			OMAP3_CORE1_IOPAD(0x210a, PIN_OUTPUT | MUX_MODE4)            /* gpio_93 (cmt_apeslpx) */
 		>;
 	};
+
+	uart2_pins: pinmux_uart2_pins {
+		pinctrl-single,pins = <
+			OMAP3_CORE1_IOPAD(0x2174, PIN_INPUT_PULLUP | MUX_MODE0)		/* uart2_cts */
+			OMAP3_CORE1_IOPAD(0x2176, PIN_OUTPUT | MUX_MODE0)		/* uart2_rts */
+			OMAP3_CORE1_IOPAD(0x2178, PIN_OUTPUT | MUX_MODE0)		/* uart2_tx */
+			OMAP3_CORE1_IOPAD(0x217a, PIN_INPUT | MUX_MODE0)		/* uart2_rx */
+		>;
+	};
 };
 
 &omap3_pmx_core2 {
@@ -435,3 +451,19 @@
 &ssi_port2 {
 	status = "disabled";
 };
+
+&uart2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart2_pins>;
+
+	bluetooth {
+		compatible = "nokia,ti,wl1271-bluetooth";
+
+		reset-gpios = <&gpio1 26 GPIO_ACTIVE_LOW>; /* 26 */
+		host-wakeup-gpios = <&gpio4 5 GPIO_ACTIVE_HIGH>; /* 101 */
+		bluetooth-wakeup-gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>; /* 37 */
+
+		clocks = <&vctcxo>;
+		clock-names = "sysclk";
+	};
+};
-- 
2.11.0

[PATCH 01/10] ARM: dts: N9/N950: add bluetooth

[Sebastian Reichel]

The Nokia N950 and N9 have a wl1271 (with nokia bootloader) bluetooth
module connected to second UART.

Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 arch/arm/boot/dts/omap3-n950-n9.dtsi | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi
index 5d8c4b4a4205..704f11c83165 100644
--- a/arch/arm/boot/dts/omap3-n950-n9.dtsi
+++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
@@ -58,6 +58,13 @@
 			pinctrl-0 = <&debug_leds>;
 		};
 	};
+
+	/* controlled (enabled/disabled) directly by wl1271 */
+	vctcxo: vctcxo {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <38400000>;
+	};
 };
 
 &omap3_pmx_core {
@@ -125,6 +132,15 @@
 			OMAP3_CORE1_IOPAD(0x210a, PIN_OUTPUT | MUX_MODE4)            /* gpio_93 (cmt_apeslpx) */
 		>;
 	};
+
+	uart2_pins: pinmux_uart2_pins {
+		pinctrl-single,pins = <
+			OMAP3_CORE1_IOPAD(0x2174, PIN_INPUT_PULLUP | MUX_MODE0)		/* uart2_cts */
+			OMAP3_CORE1_IOPAD(0x2176, PIN_OUTPUT | MUX_MODE0)		/* uart2_rts */
+			OMAP3_CORE1_IOPAD(0x2178, PIN_OUTPUT | MUX_MODE0)		/* uart2_tx */
+			OMAP3_CORE1_IOPAD(0x217a, PIN_INPUT | MUX_MODE0)		/* uart2_rx */
+		>;
+	};
 };
 
 &omap3_pmx_core2 {
@@ -435,3 +451,19 @@
 &ssi_port2 {
 	status = "disabled";
 };
+
+&uart2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart2_pins>;
+
+	bluetooth {
+		compatible = "nokia,ti,wl1271-bluetooth";
+
+		reset-gpios = <&gpio1 26 GPIO_ACTIVE_LOW>; /* 26 */
+		host-wakeup-gpios = <&gpio4 5 GPIO_ACTIVE_HIGH>; /* 101 */
+		bluetooth-wakeup-gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>; /* 37 */
+
+		clocks = <&vctcxo>;
+		clock-names = "sysclk";
+	};
+};
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/10] ARM: dts: N9/N950: add bluetooth

[Tony Lindgren]

* Sebastian Reichel <sre@kernel.org> [170304 04:00]:
> The Nokia N950 and N9 have a wl1271 (with nokia bootloader) bluetooth
> module connected to second UART.
> 
> Signed-off-by: Sebastian Reichel <sre@kernel.org>

Picking this patch into omap-for-v4.12/dt thanks.

Regards,

Tony

Re: [PATCH 01/10] ARM: dts: N9/N950: add bluetooth

[Tony Lindgren]

* Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [170304 04:00]:
> The Nokia N950 and N9 have a wl1271 (with nokia bootloader) bluetooth
> module connected to second UART.
> 
> Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Picking this patch into omap-for-v4.12/dt thanks.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/10] ARM: dts: N9/N950: add bluetooth

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 451 bytes --]

Hi!

> The Nokia N950 and N9 have a wl1271 (with nokia bootloader) bluetooth
> module connected to second UART.

I don't get it. We are describing hardware, what does it have to do
with bootloader? Is there something not described here, because it is
already initialized by bootloader?

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 01/10] ARM: dts: N9/N950: add bluetooth

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 556 bytes --]

Hi,

On Fri, Mar 17, 2017 at 04:25:29PM +0100, Pavel Machek wrote:
> Hi!
> 
> > The Nokia N950 and N9 have a wl1271 (with nokia bootloader) bluetooth
> > module connected to second UART.
> 
> I don't get it. We are describing hardware, what does it have to do
> with bootloader? Is there something not described here, because it is
> already initialized by bootloader?

This is about the wl1271 "bootloader"/low-level interface. The same
chip exists with a TI low-level interface, which requires a
different driver/firmware.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 02/10] ARM: dts: N900: Add bluetooth

[Sebastian Reichel]

Add bcm2048 node and its system clock to the N900 device tree file.
Apart from that a reference to the new clock has been added to
wl1251 (which uses it, too).

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 arch/arm/boot/dts/omap3-n900.dts | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
index b64cfda8dbb7..473230d28782 100644
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -155,6 +155,13 @@
 		compatible = "nokia,n900-ir";
 		pwms = <&pwm9 0 26316 0>; /* 38000 Hz */
 	};
+
+	/* controlled (enabled/disabled) directly by bcm2048 and wl1251 */
+	vctcxo: vctcxo {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <38400000>;
+	};
 };
 
 &omap3_pmx_core {
@@ -162,8 +169,10 @@
 
 	uart2_pins: pinmux_uart2_pins {
 		pinctrl-single,pins = <
-			OMAP3_CORE1_IOPAD(0x217a, PIN_INPUT | MUX_MODE0)		/* uart2_rx */
+			OMAP3_CORE1_IOPAD(0x2174, PIN_INPUT_PULLUP | MUX_MODE0)		/* uart2_cts */
+			OMAP3_CORE1_IOPAD(0x2176, PIN_OUTPUT | MUX_MODE0)		/* uart2_rts */
 			OMAP3_CORE1_IOPAD(0x2178, PIN_OUTPUT | MUX_MODE0)		/* uart2_tx */
+			OMAP3_CORE1_IOPAD(0x217a, PIN_INPUT | MUX_MODE0)		/* uart2_rx */
 		>;
 	};
 
@@ -920,6 +929,8 @@
 
 		interrupt-parent = <&gpio2>;
 		interrupts = <10 IRQ_TYPE_NONE>; /* gpio line 42 */
+
+		clocks = <&vctcxo>;
 	};
 };
 
@@ -937,9 +948,17 @@
 };
 
 &uart2 {
-	interrupts-extended = <&intc 73 &omap3_pmx_core OMAP3_UART2_RX>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart2_pins>;
+
+	bcm2048: bluetooth {
+		compatible = "nokia,brcm,bcm2048";
+		reset-gpios = <&gpio3 27 GPIO_ACTIVE_LOW>; /* 91 */
+		host-wakeup-gpios = <&gpio4 5 GPIO_ACTIVE_HIGH>; /* 101 */
+		bluetooth-wakeup-gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>; /* 37 */
+		clocks = <&vctcxo>;
+		clock-names = "sysclk";
+	};
 };
 
 &uart3 {
-- 
2.11.0

Re: [PATCH 02/10] ARM: dts: N900: Add bluetooth

[Tony Lindgren]

* Sebastian Reichel <sre@kernel.org> [170304 04:00]:
> Add bcm2048 node and its system clock to the N900 device tree file.
> Apart from that a reference to the new clock has been added to
> wl1251 (which uses it, too).
> 
> Signed-off-by: Sebastian Reichel <sre@kernel.org>

Picking this patch into omap-for-v4.12/dt thanks.

Regards,

Tony

Re: [PATCH 02/10] ARM: dts: N900: Add bluetooth

[Tony Lindgren]

* Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [170304 04:00]:
> Add bcm2048 node and its system clock to the N900 device tree file.
> Apart from that a reference to the new clock has been added to
> wl1251 (which uses it, too).
> 
> Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Picking this patch into omap-for-v4.12/dt thanks.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/10] ARM: dts: N900: Add bluetooth

[Rob Herring]

On Mon, Mar 6, 2017 at 10:08 AM, Tony Lindgren <tony@atomide.com> wrote:
> * Sebastian Reichel <sre@kernel.org> [170304 04:00]:
>> Add bcm2048 node and its system clock to the N900 device tree file.
>> Apart from that a reference to the new clock has been added to
>> wl1251 (which uses it, too).
>>
>> Signed-off-by: Sebastian Reichel <sre@kernel.org>
>
> Picking this patch into omap-for-v4.12/dt thanks.

The binding is not yet settled.

Rob

Re: [PATCH 02/10] ARM: dts: N900: Add bluetooth

[Rob Herring]

On Mon, Mar 6, 2017 at 10:08 AM, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:
> * Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [170304 04:00]:
>> Add bcm2048 node and its system clock to the N900 device tree file.
>> Apart from that a reference to the new clock has been added to
>> wl1251 (which uses it, too).
>>
>> Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>
> Picking this patch into omap-for-v4.12/dt thanks.

The binding is not yet settled.

Rob

Re: [PATCH 02/10] ARM: dts: N900: Add bluetooth

[Tony Lindgren]

* Rob Herring <robh+dt@kernel.org> [170307 08:34]:
> On Mon, Mar 6, 2017 at 10:08 AM, Tony Lindgren <tony@atomide.com> wrote:
> > * Sebastian Reichel <sre@kernel.org> [170304 04:00]:
> >> Add bcm2048 node and its system clock to the N900 device tree file.
> >> Apart from that a reference to the new clock has been added to
> >> wl1251 (which uses it, too).
> >>
> >> Signed-off-by: Sebastian Reichel <sre@kernel.org>
> >
> > Picking this patch into omap-for-v4.12/dt thanks.
> 
> The binding is not yet settled.

Oh sorry I thought the binding was already done. Will revert.

Regards,

Tony

Re: [PATCH 02/10] ARM: dts: N900: Add bluetooth

[Tony Lindgren]

* Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [170307 08:34]:
> On Mon, Mar 6, 2017 at 10:08 AM, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:
> > * Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [170304 04:00]:
> >> Add bcm2048 node and its system clock to the N900 device tree file.
> >> Apart from that a reference to the new clock has been added to
> >> wl1251 (which uses it, too).
> >>
> >> Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >
> > Picking this patch into omap-for-v4.12/dt thanks.
> 
> The binding is not yet settled.

Oh sorry I thought the binding was already done. Will revert.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/10] ARM: dts: N900: Add bluetooth

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 462 bytes --]

On Sat 2017-03-04 12:58:25, Sebastian Reichel wrote:
> Add bcm2048 node and its system clock to the N900 device tree file.
> Apart from that a reference to the new clock has been added to
> wl1251 (which uses it, too).
> 
> Signed-off-by: Sebastian Reichel <sre@kernel.org>

Acked-by: Pavel Machek <pavel@ucw.cz>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 02/10] ARM: dts: N900: Add bluetooth

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 513 bytes --]

On Sat 2017-03-04 12:58:25, Sebastian Reichel wrote:
> Add bcm2048 node and its system clock to the N900 device tree file.
> Apart from that a reference to the new clock has been added to
> wl1251 (which uses it, too).
> 
> Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[PATCH 03/10] Bluetooth: hci_uart: add support for word alignment

[Sebastian Reichel]

This will be used by Nokia's H4+ protocol, which
uses 2-byte aligned packets.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/bluetooth/hci_h4.c    | 17 +++++++++++++++++
 drivers/bluetooth/hci_ldisc.c |  4 ++++
 drivers/bluetooth/hci_uart.h  |  3 +++
 3 files changed, 24 insertions(+)

diff --git a/drivers/bluetooth/hci_h4.c b/drivers/bluetooth/hci_h4.c
index 635597b6e168..9c59807ebbdd 100644
--- a/drivers/bluetooth/hci_h4.c
+++ b/drivers/bluetooth/hci_h4.c
@@ -171,9 +171,20 @@ struct sk_buff *h4_recv_buf(struct hci_dev *hdev, struct sk_buff *skb,
 			    const unsigned char *buffer, int count,
 			    const struct h4_recv_pkt *pkts, int pkts_count)
 {
+	struct hci_uart *hu = hci_get_drvdata(hdev);
+	uint8_t alignment = hu->alignment;
+
 	while (count) {
 		int i, len;
 
+		/* remove padding bytes from buffer */
+		for (; hu->padding && count > 0; hu->padding--) {
+			count--;
+			buffer++;
+		}
+		if (!count)
+			break;
+
 		if (!skb) {
 			for (i = 0; i < pkts_count; i++) {
 				if (buffer[0] != (&pkts[i])->type)
@@ -253,11 +264,17 @@ struct sk_buff *h4_recv_buf(struct hci_dev *hdev, struct sk_buff *skb,
 			}
 
 			if (!dlen) {
+				hu->padding = (skb->len - 1) % alignment;
+				hu->padding = (alignment - hu->padding) % alignment;
+
 				/* No more data, complete frame */
 				(&pkts[i])->recv(hdev, skb);
 				skb = NULL;
 			}
 		} else {
+			hu->padding = (skb->len - 1) % alignment;
+			hu->padding = (alignment - hu->padding) % alignment;
+
 			/* Complete frame */
 			(&pkts[i])->recv(hdev, skb);
 			skb = NULL;
diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 9497c469efd2..0ec8a94bd712 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -459,6 +459,10 @@ static int hci_uart_tty_open(struct tty_struct *tty)
 	hu->tty = tty;
 	tty->receive_room = 65536;
 
+	/* disable alignment support by default */
+	hu->alignment = 1;
+	hu->padding = 0;
+
 	INIT_WORK(&hu->init_ready, hci_uart_init_work);
 	INIT_WORK(&hu->write_work, hci_uart_write_work);
 
diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
index 070139513e65..4aff50960cac 100644
--- a/drivers/bluetooth/hci_uart.h
+++ b/drivers/bluetooth/hci_uart.h
@@ -92,6 +92,9 @@ struct hci_uart {
 
 	unsigned int init_speed;
 	unsigned int oper_speed;
+
+	u8			alignment;
+	u8			padding;
 };
 
 /* HCI_UART proto flag bits */
-- 
2.11.0

Re: [PATCH 03/10] Bluetooth: hci_uart: add support for word alignment

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1006 bytes --]

Hi!

> This will be used by Nokia's H4+ protocol, which
> uses 2-byte aligned packets.
> 
> Signed-off-by: Sebastian Reichel <sre@kernel.org>

Acked-by: Pavel Machek <pavel@ucw.cz>

> --- a/drivers/bluetooth/hci_h4.c
> +++ b/drivers/bluetooth/hci_h4.c
> @@ -171,9 +171,20 @@ struct sk_buff *h4_recv_buf(struct hci_dev *hdev, struct sk_buff *skb,
>  			    const unsigned char *buffer, int count,
>  			    const struct h4_recv_pkt *pkts, int pkts_count)
>  {
> +	struct hci_uart *hu = hci_get_drvdata(hdev);
> +	uint8_t alignment = hu->alignment;

I'd do u8 here, too, for consistency with struct below (and becuase
uint8_t is somehow unusual in kernel).

> @@ -92,6 +92,9 @@ struct hci_uart {
>  
>  	unsigned int init_speed;
>  	unsigned int oper_speed;
> +
> +	u8			alignment;
> +	u8			padding;
>  };
>  

Thanks,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 03/10] Bluetooth: hci_uart: add support for word alignment

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1057 bytes --]

Hi!

> This will be used by Nokia's H4+ protocol, which
> uses 2-byte aligned packets.
> 
> Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>

> --- a/drivers/bluetooth/hci_h4.c
> +++ b/drivers/bluetooth/hci_h4.c
> @@ -171,9 +171,20 @@ struct sk_buff *h4_recv_buf(struct hci_dev *hdev, struct sk_buff *skb,
>  			    const unsigned char *buffer, int count,
>  			    const struct h4_recv_pkt *pkts, int pkts_count)
>  {
> +	struct hci_uart *hu = hci_get_drvdata(hdev);
> +	uint8_t alignment = hu->alignment;

I'd do u8 here, too, for consistency with struct below (and becuase
uint8_t is somehow unusual in kernel).

> @@ -92,6 +92,9 @@ struct hci_uart {
>  
>  	unsigned int init_speed;
>  	unsigned int oper_speed;
> +
> +	u8			alignment;
> +	u8			padding;
>  };
>  

Thanks,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[PATCH 04/10] Bluetooth: hci_uart: add serdev driver support library

[Sebastian Reichel]

From: Rob Herring <robh@kernel.org>

This adds library functions for serdev based BT drivers. This is largely
copied from hci_ldisc.c and modified to use serdev calls. There's a little
bit of duplication, but I avoided intermixing this as the ldisc code should
eventually go away.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Gustavo Padovan <gustavo@padovan.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>
Cc: linux-bluetooth@vger.kernel.org
Tested-By: Sebastian Reichel <sre@kernel.org>
---
 drivers/bluetooth/Makefile     |   1 +
 drivers/bluetooth/hci_serdev.c | 370 +++++++++++++++++++++++++++++++++++++++++
 drivers/bluetooth/hci_uart.h   |   4 +
 3 files changed, 375 insertions(+)
 create mode 100644 drivers/bluetooth/hci_serdev.c

diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
index 80627187c8b6..fd571689eed6 100644
--- a/drivers/bluetooth/Makefile
+++ b/drivers/bluetooth/Makefile
@@ -29,6 +29,7 @@ btmrvl-y			:= btmrvl_main.o
 btmrvl-$(CONFIG_DEBUG_FS)	+= btmrvl_debugfs.o
 
 hci_uart-y				:= hci_ldisc.o
+hci_uart-$(CONFIG_SERIAL_DEV_BUS)	+= hci_serdev.o
 hci_uart-$(CONFIG_BT_HCIUART_H4)	+= hci_h4.o
 hci_uart-$(CONFIG_BT_HCIUART_BCSP)	+= hci_bcsp.o
 hci_uart-$(CONFIG_BT_HCIUART_LL)	+= hci_ll.o
diff --git a/drivers/bluetooth/hci_serdev.c b/drivers/bluetooth/hci_serdev.c
new file mode 100644
index 000000000000..b2f7293a3162
--- /dev/null
+++ b/drivers/bluetooth/hci_serdev.c
@@ -0,0 +1,370 @@
+/*
+ *
+ *  Bluetooth HCI serdev driver lib
+ *
+ *  Copyright (C) 2017  Linaro, Ltd., Rob Herring <robh@kernel.org>
+ *
+ *  Based on hci_ldisc.c:
+ *
+ *  Copyright (C) 2000-2001  Qualcomm Incorporated
+ *  Copyright (C) 2002-2003  Maxim Krasnyansky <maxk@qualcomm.com>
+ *  Copyright (C) 2004-2005  Marcel Holtmann <marcel@holtmann.org>
+ *
+ *
+ *  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.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/serdev.h>
+#include <linux/skbuff.h>
+
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci_core.h>
+
+#include "hci_uart.h"
+
+struct serdev_device_ops hci_serdev_client_ops;
+
+static inline void hci_uart_tx_complete(struct hci_uart *hu, int pkt_type)
+{
+	struct hci_dev *hdev = hu->hdev;
+
+	/* Update HCI stat counters */
+	switch (pkt_type) {
+	case HCI_COMMAND_PKT:
+		hdev->stat.cmd_tx++;
+		break;
+
+	case HCI_ACLDATA_PKT:
+		hdev->stat.acl_tx++;
+		break;
+
+	case HCI_SCODATA_PKT:
+		hdev->stat.sco_tx++;
+		break;
+	}
+}
+
+static inline struct sk_buff *hci_uart_dequeue(struct hci_uart *hu)
+{
+	struct sk_buff *skb = hu->tx_skb;
+
+	if (!skb)
+		skb = hu->proto->dequeue(hu);
+	else
+		hu->tx_skb = NULL;
+
+	return skb;
+}
+
+static void hci_uart_write_work(struct work_struct *work)
+{
+	struct hci_uart *hu = container_of(work, struct hci_uart, write_work);
+	struct serdev_device *serdev = hu->serdev;
+	struct hci_dev *hdev = hu->hdev;
+	struct sk_buff *skb;
+
+	/* REVISIT: should we cope with bad skbs or ->write() returning
+	 * and error value ?
+	 */
+
+restart:
+	clear_bit(HCI_UART_TX_WAKEUP, &hu->tx_state);
+
+	while ((skb = hci_uart_dequeue(hu))) {
+		int len;
+
+		len = serdev_device_write_buf(serdev, skb->data, skb->len);
+		hdev->stat.byte_tx += len;
+
+		skb_pull(skb, len);
+		if (skb->len) {
+			hu->tx_skb = skb;
+			break;
+		}
+
+		hci_uart_tx_complete(hu, hci_skb_pkt_type(skb));
+		kfree_skb(skb);
+	}
+
+	if (test_bit(HCI_UART_TX_WAKEUP, &hu->tx_state))
+		goto restart;
+
+	clear_bit(HCI_UART_SENDING, &hu->tx_state);
+}
+
+/* ------- Interface to HCI layer ------ */
+/* Initialize device */
+static int hci_uart_open(struct hci_dev *hdev)
+{
+	struct hci_uart *hu  = hci_get_drvdata(hdev);
+
+	BT_DBG("%s %p", hdev->name, hdev);
+
+	serdev_device_set_client_ops(hu->serdev, &hci_serdev_client_ops);
+
+	return serdev_device_open(hu->serdev);
+}
+
+/* Reset device */
+static int hci_uart_flush(struct hci_dev *hdev)
+{
+	struct hci_uart *hu  = hci_get_drvdata(hdev);
+
+	BT_DBG("hdev %p serdev %p", hdev, hu->serdev);
+
+	if (hu->tx_skb) {
+		kfree_skb(hu->tx_skb); hu->tx_skb = NULL;
+	}
+
+	/* Flush any pending characters in the driver and discipline. */
+	serdev_device_write_flush(hu->serdev);
+
+	if (test_bit(HCI_UART_PROTO_READY, &hu->flags))
+		hu->proto->flush(hu);
+
+	return 0;
+}
+
+/* Close device */
+static int hci_uart_close(struct hci_dev *hdev)
+{
+	struct hci_uart *hu  = hci_get_drvdata(hdev);
+
+	BT_DBG("hdev %p", hdev);
+
+	hci_uart_flush(hdev);
+	hdev->flush = NULL;
+
+	serdev_device_close(hu->serdev);
+
+	return 0;
+}
+
+/* Send frames from HCI layer */
+static int hci_uart_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	struct hci_uart *hu = hci_get_drvdata(hdev);
+
+	BT_DBG("%s: type %d len %d", hdev->name, hci_skb_pkt_type(skb),
+	       skb->len);
+
+	hu->proto->enqueue(hu, skb);
+
+	hci_uart_tx_wakeup(hu);
+
+	return 0;
+}
+
+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;
+	unsigned int speed;
+	int err;
+
+	/* Init speed if any */
+	if (hu->init_speed)
+		speed = hu->init_speed;
+	else if (hu->proto->init_speed)
+		speed = hu->proto->init_speed;
+	else
+		speed = 0;
+
+	if (speed)
+		serdev_device_set_baudrate(hu->serdev, speed);
+
+	/* Operational speed if any */
+	if (hu->oper_speed)
+		speed = hu->oper_speed;
+	else if (hu->proto->oper_speed)
+		speed = hu->proto->oper_speed;
+	else
+		speed = 0;
+
+	if (hu->proto->set_baudrate && speed) {
+		err = hu->proto->set_baudrate(hu, speed);
+		if (!err)
+			serdev_device_set_baudrate(hu->serdev, speed);
+	}
+
+	if (hu->proto->setup)
+		return hu->proto->setup(hu);
+
+	if (!test_bit(HCI_UART_VND_DETECT, &hu->hdev_flags))
+		return 0;
+
+	skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_VERSION, 0, NULL,
+			     HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		BT_ERR("%s: Reading local version information failed (%ld)",
+		       hdev->name, PTR_ERR(skb));
+		return 0;
+	}
+
+	if (skb->len != sizeof(*ver)) {
+		BT_ERR("%s: Event length mismatch for version information",
+		       hdev->name);
+		goto done;
+	}
+
+done:
+	kfree_skb(skb);
+	return 0;
+}
+
+/* hci_uart_write_wakeup()
+ *
+ *    Callback for transmit wakeup. Called when low level
+ *    device driver can accept more send data.
+ *
+ * Arguments:        tty    pointer to associated tty instance data
+ * Return Value:    None
+ */
+static void hci_uart_write_wakeup(struct serdev_device *serdev)
+{
+	struct hci_uart *hu = serdev_device_get_drvdata(serdev);
+
+	BT_DBG("");
+
+	if (!hu)
+		return;
+
+	if (serdev != hu->serdev)
+		return;
+
+	if (test_bit(HCI_UART_PROTO_READY, &hu->flags))
+		hci_uart_tx_wakeup(hu);
+}
+
+/* hci_uart_receive_buf()
+ *
+ *     Called by tty low level driver when receive data is
+ *     available.
+ *
+ * Arguments:  tty          pointer to tty isntance data
+ *             data         pointer to received data
+ *             flags        pointer to flags for data
+ *             count        count of received data in bytes
+ *
+ * Return Value:    None
+ */
+static int hci_uart_receive_buf(struct serdev_device *serdev, const u8 *data,
+				   size_t count)
+{
+	struct hci_uart *hu = serdev_device_get_drvdata(serdev);
+
+	if (!hu || serdev != hu->serdev)
+		return 0;
+
+	if (!test_bit(HCI_UART_PROTO_READY, &hu->flags))
+		return 0;
+
+	/* It does not need a lock here as it is already protected by a mutex in
+	 * tty caller
+	 */
+	hu->proto->recv(hu, data, count);
+
+	if (hu->hdev)
+		hu->hdev->stat.byte_rx += count;
+
+	return count;
+}
+
+struct serdev_device_ops hci_serdev_client_ops = {
+	.receive_buf = hci_uart_receive_buf,
+	.write_wakeup = hci_uart_write_wakeup,
+};
+
+int hci_uart_register_device(struct hci_uart *hu, const struct hci_uart_proto *p)
+{
+	int err;
+	struct hci_dev *hdev;
+
+	BT_DBG("");
+
+	err = p->open(hu);
+	if (err)
+		return err;
+
+	hu->proto = p;
+	set_bit(HCI_UART_PROTO_READY, &hu->flags);
+
+	/* Initialize and register HCI device */
+	hdev = hci_alloc_dev();
+	if (!hdev) {
+		BT_ERR("Can't allocate HCI device");
+		err = -ENOMEM;
+		goto err_alloc;
+	}
+
+	hu->hdev = hdev;
+
+	hdev->bus = HCI_UART;
+	hci_set_drvdata(hdev, hu);
+
+	INIT_WORK(&hu->write_work, hci_uart_write_work);
+
+	/* Only when vendor specific setup callback is provided, consider
+	 * the manufacturer information valid. This avoids filling in the
+	 * value for Ericsson when nothing is specified.
+	 */
+	if (hu->proto->setup)
+		hdev->manufacturer = hu->proto->manufacturer;
+
+	hdev->open  = hci_uart_open;
+	hdev->close = hci_uart_close;
+	hdev->flush = hci_uart_flush;
+	hdev->send  = hci_uart_send_frame;
+	hdev->setup = hci_uart_setup;
+	SET_HCIDEV_DEV(hdev, &hu->serdev->dev);
+
+	if (test_bit(HCI_UART_RAW_DEVICE, &hu->hdev_flags))
+		set_bit(HCI_QUIRK_RAW_DEVICE, &hdev->quirks);
+
+	if (test_bit(HCI_UART_EXT_CONFIG, &hu->hdev_flags))
+		set_bit(HCI_QUIRK_EXTERNAL_CONFIG, &hdev->quirks);
+
+	if (!test_bit(HCI_UART_RESET_ON_INIT, &hu->hdev_flags))
+		set_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks);
+
+	if (test_bit(HCI_UART_CREATE_AMP, &hu->hdev_flags))
+		hdev->dev_type = HCI_AMP;
+	else
+		hdev->dev_type = HCI_PRIMARY;
+
+	if (test_bit(HCI_UART_INIT_PENDING, &hu->hdev_flags))
+		return 0;
+
+	if (hci_register_dev(hdev) < 0) {
+		BT_ERR("Can't register HCI device");
+		err = -ENODEV;
+		goto err_register;
+	}
+
+	set_bit(HCI_UART_REGISTERED, &hu->flags);
+
+	return 0;
+
+err_register:
+	hci_free_dev(hdev);
+err_alloc:
+	clear_bit(HCI_UART_PROTO_READY, &hu->flags);
+	p->close(hu);
+	return err;
+}
diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
index 4aff50960cac..1b41c661bbb8 100644
--- a/drivers/bluetooth/hci_uart.h
+++ b/drivers/bluetooth/hci_uart.h
@@ -58,6 +58,7 @@
 #define HCI_UART_VND_DETECT	5
 
 struct hci_uart;
+struct serdev_device;
 
 struct hci_uart_proto {
 	unsigned int id;
@@ -77,6 +78,7 @@ struct hci_uart_proto {
 
 struct hci_uart {
 	struct tty_struct	*tty;
+	struct serdev_device	*serdev;
 	struct hci_dev		*hdev;
 	unsigned long		flags;
 	unsigned long		hdev_flags;
@@ -108,6 +110,8 @@ struct hci_uart {
 
 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_register_device(struct hci_uart *hu, 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_init_tty(struct hci_uart *hu);
-- 
2.11.0

Re: [PATCH 04/10] Bluetooth: hci_uart: add serdev driver support library

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 3459 bytes --]

Hi!

> From: Rob Herring <robh@kernel.org>
> 
> This adds library functions for serdev based BT drivers. This is largely
> copied from hci_ldisc.c and modified to use serdev calls. There's a little
> bit of duplication, but I avoided intermixing this as the ldisc code should
> eventually go away.
> 
> Signed-off-by: Rob Herring <robh@kernel.org>
> Cc: Marcel Holtmann <marcel@holtmann.org>
> Cc: Gustavo Padovan <gustavo@padovan.org>
> Cc: Johan Hedberg <johan.hedberg@gmail.com>
> Cc: linux-bluetooth@vger.kernel.org
> Tested-By: Sebastian Reichel <sre@kernel.org>

Trivial comments below.

> @@ -0,0 +1,370 @@
> +/*
> + *
> + *  Bluetooth HCI serdev driver lib

Just one empty line.

> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + *
> + */

I don't think we put the address in there any more.

> +static void hci_uart_write_work(struct work_struct *work)
> +{
> +	struct hci_uart *hu = container_of(work, struct hci_uart, write_work);
> +	struct serdev_device *serdev = hu->serdev;
> +	struct hci_dev *hdev = hu->hdev;
> +	struct sk_buff *skb;
> +
> +	/* REVISIT: should we cope with bad skbs or ->write() returning
> +	 * and error value ?
> +	 */

"an error value"? Comment style?

> +restart:
> +	clear_bit(HCI_UART_TX_WAKEUP, &hu->tx_state);
> +
> +	while ((skb = hci_uart_dequeue(hu))) {
> +		int len;
> +
> +		len = serdev_device_write_buf(serdev, skb->data, skb->len);
> +		hdev->stat.byte_tx += len;
> +
> +		skb_pull(skb, len);
> +		if (skb->len) {
> +			hu->tx_skb = skb;
> +			break;
> +		}
> +
> +		hci_uart_tx_complete(hu, hci_skb_pkt_type(skb));
> +		kfree_skb(skb);
> +	}
> +
> +	if (test_bit(HCI_UART_TX_WAKEUP, &hu->tx_state))
> +		goto restart;

do {} while () instead of goto?

> +/* ------- Interface to HCI layer ------ */
> +/* Initialize device */

Comment style?

> +	if (hu->proto->set_baudrate && speed) {
> +		err = hu->proto->set_baudrate(hu, speed);
> +		if (!err)
> +			serdev_device_set_baudrate(hu->serdev, speed);
> +	}

Complain about the error here?

> +	if (skb->len != sizeof(*ver)) {
> +		BT_ERR("%s: Event length mismatch for version information",
> +		       hdev->name);
> +		goto done;
> +	}
> +
> +done:

Get rid of goto and label?

> +/* hci_uart_write_wakeup()
> + *
> + *    Callback for transmit wakeup. Called when low level
> + *    device driver can accept more send data.
> + *
> + * Arguments:        tty    pointer to associated tty instance data
> + * Return Value:    None
> + */

Kerneldoc? :-)

> +static void hci_uart_write_wakeup(struct serdev_device *serdev)
> +{
> +	struct hci_uart *hu = serdev_device_get_drvdata(serdev);
> +
> +	BT_DBG("");
> +
> +	if (!hu)
> +		return;
> +
> +	if (serdev != hu->serdev)
> +		return;

WARN_ON on both of these? That should not be normal condition...

> +	/* Only when vendor specific setup callback is provided, consider
> +	 * the manufacturer information valid. This avoids filling in the
> +	 * value for Ericsson when nothing is specified.
> +	 */

Is this comment still up-to-date?

Thanks,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[PATCH 05/10] Bluetooth: hci_serdev: do not open device in hci open

[Sebastian Reichel]

The device driver may need to communicate with the UART
device while the Bluetooth device is closed (e.g. due
to interrupts).

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/bluetooth/hci_serdev.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/bluetooth/hci_serdev.c b/drivers/bluetooth/hci_serdev.c
index b2f7293a3162..b396d13a7b50 100644
--- a/drivers/bluetooth/hci_serdev.c
+++ b/drivers/bluetooth/hci_serdev.c
@@ -111,13 +111,9 @@ static void hci_uart_write_work(struct work_struct *work)
 /* Initialize device */
 static int hci_uart_open(struct hci_dev *hdev)
 {
-	struct hci_uart *hu  = hci_get_drvdata(hdev);
-
 	BT_DBG("%s %p", hdev->name, hdev);
 
-	serdev_device_set_client_ops(hu->serdev, &hci_serdev_client_ops);
-
-	return serdev_device_open(hu->serdev);
+	return 0;
 }
 
 /* Reset device */
@@ -143,15 +139,11 @@ static int hci_uart_flush(struct hci_dev *hdev)
 /* Close device */
 static int hci_uart_close(struct hci_dev *hdev)
 {
-	struct hci_uart *hu  = hci_get_drvdata(hdev);
-
 	BT_DBG("hdev %p", hdev);
 
 	hci_uart_flush(hdev);
 	hdev->flush = NULL;
 
-	serdev_device_close(hu->serdev);
-
 	return 0;
 }
 
@@ -298,6 +290,8 @@ int hci_uart_register_device(struct hci_uart *hu, const struct hci_uart_proto *p
 
 	BT_DBG("");
 
+	serdev_device_set_client_ops(hu->serdev, &hci_serdev_client_ops);
+
 	err = p->open(hu);
 	if (err)
 		return err;
@@ -366,5 +360,6 @@ int hci_uart_register_device(struct hci_uart *hu, const struct hci_uart_proto *p
 err_alloc:
 	clear_bit(HCI_UART_PROTO_READY, &hu->flags);
 	p->close(hu);
+
 	return err;
 }
-- 
2.11.0

Re: [PATCH 05/10] Bluetooth: hci_serdev: do not open device in hci open

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 451 bytes --]

> @@ -366,5 +360,6 @@ int hci_uart_register_device(struct hci_uart *hu, const struct hci_uart_proto *p
>  err_alloc:
>  	clear_bit(HCI_UART_PROTO_READY, &hu->flags);
>  	p->close(hu);
> +
>  	return err;
>  }

This could be removed, I guess.

Acked-by: Pavel Machek <pavel@ucw.cz>

									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[PATCH 06/10] tty: serial: omap: add UPF_BOOT_AUTOCONF flag for DT init

[Sebastian Reichel]

The UPF_BOOT_AUTOCONF flag is needed for proper
flow control support.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/tty/serial/omap-serial.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 6c6f82ad8d5c..a4734649a0f0 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1597,6 +1597,9 @@ static struct omap_uart_port_info *of_get_uart_port_info(struct device *dev)
 
 	of_property_read_u32(dev->of_node, "clock-frequency",
 					 &omap_up_info->uartclk);
+
+	omap_up_info->flags = UPF_BOOT_AUTOCONF;
+
 	return omap_up_info;
 }
 
-- 
2.11.0

[PATCH 06/10] tty: serial: omap: add UPF_BOOT_AUTOCONF flag for DT init

[Sebastian Reichel]

The UPF_BOOT_AUTOCONF flag is needed for proper
flow control support.

Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/tty/serial/omap-serial.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 6c6f82ad8d5c..a4734649a0f0 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1597,6 +1597,9 @@ static struct omap_uart_port_info *of_get_uart_port_info(struct device *dev)
 
 	of_property_read_u32(dev->of_node, "clock-frequency",
 					 &omap_up_info->uartclk);
+
+	omap_up_info->flags = UPF_BOOT_AUTOCONF;
+
 	return omap_up_info;
 }
 
-- 
2.11.0

[PATCH 07/10] serdev: add serdev_device_wait_until_sent

[Sebastian Reichel]

Add method, which waits until the transmission buffer has been sent.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/tty/serdev/core.c           | 11 +++++++++++
 drivers/tty/serdev/serdev-ttyport.c | 15 ++++++++++++++-
 include/linux/serdev.h              |  3 +++
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index f4c6c90add78..a63b74031e22 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -173,6 +173,17 @@ void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable)
 }
 EXPORT_SYMBOL_GPL(serdev_device_set_flow_control);
 
+void serdev_device_wait_until_sent(struct serdev_device *serdev, long timeout)
+{
+	struct serdev_controller *ctrl = serdev->ctrl;
+
+	if (!ctrl || !ctrl->ops->wait_until_sent)
+		return;
+
+	ctrl->ops->wait_until_sent(ctrl, timeout);
+}
+EXPORT_SYMBOL_GPL(serdev_device_wait_until_sent);
+
 static int serdev_drv_probe(struct device *dev)
 {
 	const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
index d05393594f15..db2bc601e554 100644
--- a/drivers/tty/serdev/serdev-ttyport.c
+++ b/drivers/tty/serdev/serdev-ttyport.c
@@ -14,6 +14,7 @@
 #include <linux/serdev.h>
 #include <linux/tty.h>
 #include <linux/tty_driver.h>
+#include <linux/poll.h>
 
 #define SERPORT_ACTIVE		1
 
@@ -47,10 +48,13 @@ static void ttyport_write_wakeup(struct tty_port *port)
 	struct serport *serport = serdev_controller_get_drvdata(ctrl);
 
 	if (!test_and_clear_bit(TTY_DO_WRITE_WAKEUP, &port->tty->flags))
-		return;
+		goto out;
 
 	if (test_bit(SERPORT_ACTIVE, &serport->flags))
 		serdev_controller_write_wakeup(ctrl);
+
+out:
+	wake_up_interruptible_poll(&port->tty->write_wait, POLLOUT);
 }
 
 static const struct tty_port_client_operations client_ops = {
@@ -167,6 +171,14 @@ static void ttyport_set_flow_control(struct serdev_controller *ctrl, bool enable
 	tty_set_termios(tty, &ktermios);
 }
 
+static void ttyport_wait_until_sent(struct serdev_controller *ctrl, long timeout)
+{
+	struct serport *serport = serdev_controller_get_drvdata(ctrl);
+	struct tty_struct *tty = serport->tty;
+
+	tty_wait_until_sent(tty, timeout);
+}
+
 static const struct serdev_controller_ops ctrl_ops = {
 	.write_buf = ttyport_write_buf,
 	.write_flush = ttyport_write_flush,
@@ -175,6 +187,7 @@ static const struct serdev_controller_ops ctrl_ops = {
 	.close = ttyport_close,
 	.set_flow_control = ttyport_set_flow_control,
 	.set_baudrate = ttyport_set_baudrate,
+	.wait_until_sent = ttyport_wait_until_sent,
 };
 
 struct device *serdev_tty_port_register(struct tty_port *port,
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index 9519da6253a8..a308b206d204 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -81,6 +81,7 @@ struct serdev_controller_ops {
 	void (*close)(struct serdev_controller *);
 	void (*set_flow_control)(struct serdev_controller *, bool);
 	unsigned int (*set_baudrate)(struct serdev_controller *, unsigned int);
+	void (*wait_until_sent)(struct serdev_controller *, long);
 };
 
 /**
@@ -186,6 +187,7 @@ int serdev_device_open(struct serdev_device *);
 void serdev_device_close(struct serdev_device *);
 unsigned int serdev_device_set_baudrate(struct serdev_device *, unsigned int);
 void serdev_device_set_flow_control(struct serdev_device *, bool);
+void serdev_device_wait_until_sent(struct serdev_device *, long);
 int serdev_device_write_buf(struct serdev_device *, const unsigned char *, size_t);
 void serdev_device_write_flush(struct serdev_device *);
 int serdev_device_write_room(struct serdev_device *);
@@ -223,6 +225,7 @@ static inline unsigned int serdev_device_set_baudrate(struct serdev_device *sdev
 	return 0;
 }
 static inline void serdev_device_set_flow_control(struct serdev_device *sdev, bool enable) {}
+static inline void serdev_device_wait_until_sent(struct serdev_device *sdev, long timeout) {}
 static inline int serdev_device_write_buf(struct serdev_device *sdev, const unsigned char *buf, size_t count)
 {
 	return -ENODEV;
-- 
2.11.0

[PATCH 07/10] serdev: add serdev_device_wait_until_sent

[Sebastian Reichel]

Add method, which waits until the transmission buffer has been sent.

Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/tty/serdev/core.c           | 11 +++++++++++
 drivers/tty/serdev/serdev-ttyport.c | 15 ++++++++++++++-
 include/linux/serdev.h              |  3 +++
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index f4c6c90add78..a63b74031e22 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -173,6 +173,17 @@ void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable)
 }
 EXPORT_SYMBOL_GPL(serdev_device_set_flow_control);
 
+void serdev_device_wait_until_sent(struct serdev_device *serdev, long timeout)
+{
+	struct serdev_controller *ctrl = serdev->ctrl;
+
+	if (!ctrl || !ctrl->ops->wait_until_sent)
+		return;
+
+	ctrl->ops->wait_until_sent(ctrl, timeout);
+}
+EXPORT_SYMBOL_GPL(serdev_device_wait_until_sent);
+
 static int serdev_drv_probe(struct device *dev)
 {
 	const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
index d05393594f15..db2bc601e554 100644
--- a/drivers/tty/serdev/serdev-ttyport.c
+++ b/drivers/tty/serdev/serdev-ttyport.c
@@ -14,6 +14,7 @@
 #include <linux/serdev.h>
 #include <linux/tty.h>
 #include <linux/tty_driver.h>
+#include <linux/poll.h>
 
 #define SERPORT_ACTIVE		1
 
@@ -47,10 +48,13 @@ static void ttyport_write_wakeup(struct tty_port *port)
 	struct serport *serport = serdev_controller_get_drvdata(ctrl);
 
 	if (!test_and_clear_bit(TTY_DO_WRITE_WAKEUP, &port->tty->flags))
-		return;
+		goto out;
 
 	if (test_bit(SERPORT_ACTIVE, &serport->flags))
 		serdev_controller_write_wakeup(ctrl);
+
+out:
+	wake_up_interruptible_poll(&port->tty->write_wait, POLLOUT);
 }
 
 static const struct tty_port_client_operations client_ops = {
@@ -167,6 +171,14 @@ static void ttyport_set_flow_control(struct serdev_controller *ctrl, bool enable
 	tty_set_termios(tty, &ktermios);
 }
 
+static void ttyport_wait_until_sent(struct serdev_controller *ctrl, long timeout)
+{
+	struct serport *serport = serdev_controller_get_drvdata(ctrl);
+	struct tty_struct *tty = serport->tty;
+
+	tty_wait_until_sent(tty, timeout);
+}
+
 static const struct serdev_controller_ops ctrl_ops = {
 	.write_buf = ttyport_write_buf,
 	.write_flush = ttyport_write_flush,
@@ -175,6 +187,7 @@ static const struct serdev_controller_ops ctrl_ops = {
 	.close = ttyport_close,
 	.set_flow_control = ttyport_set_flow_control,
 	.set_baudrate = ttyport_set_baudrate,
+	.wait_until_sent = ttyport_wait_until_sent,
 };
 
 struct device *serdev_tty_port_register(struct tty_port *port,
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index 9519da6253a8..a308b206d204 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -81,6 +81,7 @@ struct serdev_controller_ops {
 	void (*close)(struct serdev_controller *);
 	void (*set_flow_control)(struct serdev_controller *, bool);
 	unsigned int (*set_baudrate)(struct serdev_controller *, unsigned int);
+	void (*wait_until_sent)(struct serdev_controller *, long);
 };
 
 /**
@@ -186,6 +187,7 @@ int serdev_device_open(struct serdev_device *);
 void serdev_device_close(struct serdev_device *);
 unsigned int serdev_device_set_baudrate(struct serdev_device *, unsigned int);
 void serdev_device_set_flow_control(struct serdev_device *, bool);
+void serdev_device_wait_until_sent(struct serdev_device *, long);
 int serdev_device_write_buf(struct serdev_device *, const unsigned char *, size_t);
 void serdev_device_write_flush(struct serdev_device *);
 int serdev_device_write_room(struct serdev_device *);
@@ -223,6 +225,7 @@ static inline unsigned int serdev_device_set_baudrate(struct serdev_device *sdev
 	return 0;
 }
 static inline void serdev_device_set_flow_control(struct serdev_device *sdev, bool enable) {}
+static inline void serdev_device_wait_until_sent(struct serdev_device *sdev, long timeout) {}
 static inline int serdev_device_write_buf(struct serdev_device *sdev, const unsigned char *buf, size_t count)
 {
 	return -ENODEV;
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/10] serdev: add serdev_device_wait_until_sent

[Rob Herring]

On Sat, Mar 4, 2017 at 5:58 AM, Sebastian Reichel <sre@kernel.org> wrote:
> Add method, which waits until the transmission buffer has been sent.
>
> Signed-off-by: Sebastian Reichel <sre@kernel.org>
> ---
>  drivers/tty/serdev/core.c           | 11 +++++++++++
>  drivers/tty/serdev/serdev-ttyport.c | 15 ++++++++++++++-
>  include/linux/serdev.h              |  3 +++
>  3 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index f4c6c90add78..a63b74031e22 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -173,6 +173,17 @@ void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable)
>  }
>  EXPORT_SYMBOL_GPL(serdev_device_set_flow_control);
>
> +void serdev_device_wait_until_sent(struct serdev_device *serdev, long timeout)
> +{
> +       struct serdev_controller *ctrl = serdev->ctrl;
> +
> +       if (!ctrl || !ctrl->ops->wait_until_sent)
> +               return;
> +
> +       ctrl->ops->wait_until_sent(ctrl, timeout);
> +}
> +EXPORT_SYMBOL_GPL(serdev_device_wait_until_sent);
> +
>  static int serdev_drv_probe(struct device *dev)
>  {
>         const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
> diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
> index d05393594f15..db2bc601e554 100644
> --- a/drivers/tty/serdev/serdev-ttyport.c
> +++ b/drivers/tty/serdev/serdev-ttyport.c
> @@ -14,6 +14,7 @@
>  #include <linux/serdev.h>
>  #include <linux/tty.h>
>  #include <linux/tty_driver.h>
> +#include <linux/poll.h>
>
>  #define SERPORT_ACTIVE         1
>
> @@ -47,10 +48,13 @@ static void ttyport_write_wakeup(struct tty_port *port)
>         struct serport *serport = serdev_controller_get_drvdata(ctrl);
>
>         if (!test_and_clear_bit(TTY_DO_WRITE_WAKEUP, &port->tty->flags))
> -               return;
> +               goto out;
>
>         if (test_bit(SERPORT_ACTIVE, &serport->flags))
>                 serdev_controller_write_wakeup(ctrl);
> +
> +out:
> +       wake_up_interruptible_poll(&port->tty->write_wait, POLLOUT);

Shouldn't this be separate patch?

Rob

Re: [PATCH 07/10] serdev: add serdev_device_wait_until_sent

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 2563 bytes --]

Hi,

On Tue, Mar 07, 2017 at 09:34:49AM -0600, Rob Herring wrote:
> On Sat, Mar 4, 2017 at 5:58 AM, Sebastian Reichel <sre@kernel.org> wrote:
> > Add method, which waits until the transmission buffer has been sent.
> >
> > Signed-off-by: Sebastian Reichel <sre@kernel.org>
> > ---
> >  drivers/tty/serdev/core.c           | 11 +++++++++++
> >  drivers/tty/serdev/serdev-ttyport.c | 15 ++++++++++++++-
> >  include/linux/serdev.h              |  3 +++
> >  3 files changed, 28 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> > index f4c6c90add78..a63b74031e22 100644
> > --- a/drivers/tty/serdev/core.c
> > +++ b/drivers/tty/serdev/core.c
> > @@ -173,6 +173,17 @@ void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable)
> >  }
> >  EXPORT_SYMBOL_GPL(serdev_device_set_flow_control);
> >
> > +void serdev_device_wait_until_sent(struct serdev_device *serdev, long timeout)
> > +{
> > +       struct serdev_controller *ctrl = serdev->ctrl;
> > +
> > +       if (!ctrl || !ctrl->ops->wait_until_sent)
> > +               return;
> > +
> > +       ctrl->ops->wait_until_sent(ctrl, timeout);
> > +}
> > +EXPORT_SYMBOL_GPL(serdev_device_wait_until_sent);
> > +
> >  static int serdev_drv_probe(struct device *dev)
> >  {
> >         const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
> > diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
> > index d05393594f15..db2bc601e554 100644
> > --- a/drivers/tty/serdev/serdev-ttyport.c
> > +++ b/drivers/tty/serdev/serdev-ttyport.c
> > @@ -14,6 +14,7 @@
> >  #include <linux/serdev.h>
> >  #include <linux/tty.h>
> >  #include <linux/tty_driver.h>
> > +#include <linux/poll.h>
> >
> >  #define SERPORT_ACTIVE         1
> >
> > @@ -47,10 +48,13 @@ static void ttyport_write_wakeup(struct tty_port *port)
> >         struct serport *serport = serdev_controller_get_drvdata(ctrl);
> >
> >         if (!test_and_clear_bit(TTY_DO_WRITE_WAKEUP, &port->tty->flags))
> > -               return;
> > +               goto out;
> >
> >         if (test_bit(SERPORT_ACTIVE, &serport->flags))
> >                 serdev_controller_write_wakeup(ctrl);
> > +
> > +out:
> > +       wake_up_interruptible_poll(&port->tty->write_wait, POLLOUT);
> 
> Shouldn't this be separate patch?

That change is needed for tty_wait_until_sent to work correctly, which
starts with a wait_event_interruptible_timeout() on write_wait.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 07/10] serdev: add serdev_device_wait_until_sent

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 2621 bytes --]

Hi,

On Tue, Mar 07, 2017 at 09:34:49AM -0600, Rob Herring wrote:
> On Sat, Mar 4, 2017 at 5:58 AM, Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > Add method, which waits until the transmission buffer has been sent.
> >
> > Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > ---
> >  drivers/tty/serdev/core.c           | 11 +++++++++++
> >  drivers/tty/serdev/serdev-ttyport.c | 15 ++++++++++++++-
> >  include/linux/serdev.h              |  3 +++
> >  3 files changed, 28 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> > index f4c6c90add78..a63b74031e22 100644
> > --- a/drivers/tty/serdev/core.c
> > +++ b/drivers/tty/serdev/core.c
> > @@ -173,6 +173,17 @@ void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable)
> >  }
> >  EXPORT_SYMBOL_GPL(serdev_device_set_flow_control);
> >
> > +void serdev_device_wait_until_sent(struct serdev_device *serdev, long timeout)
> > +{
> > +       struct serdev_controller *ctrl = serdev->ctrl;
> > +
> > +       if (!ctrl || !ctrl->ops->wait_until_sent)
> > +               return;
> > +
> > +       ctrl->ops->wait_until_sent(ctrl, timeout);
> > +}
> > +EXPORT_SYMBOL_GPL(serdev_device_wait_until_sent);
> > +
> >  static int serdev_drv_probe(struct device *dev)
> >  {
> >         const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
> > diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
> > index d05393594f15..db2bc601e554 100644
> > --- a/drivers/tty/serdev/serdev-ttyport.c
> > +++ b/drivers/tty/serdev/serdev-ttyport.c
> > @@ -14,6 +14,7 @@
> >  #include <linux/serdev.h>
> >  #include <linux/tty.h>
> >  #include <linux/tty_driver.h>
> > +#include <linux/poll.h>
> >
> >  #define SERPORT_ACTIVE         1
> >
> > @@ -47,10 +48,13 @@ static void ttyport_write_wakeup(struct tty_port *port)
> >         struct serport *serport = serdev_controller_get_drvdata(ctrl);
> >
> >         if (!test_and_clear_bit(TTY_DO_WRITE_WAKEUP, &port->tty->flags))
> > -               return;
> > +               goto out;
> >
> >         if (test_bit(SERPORT_ACTIVE, &serport->flags))
> >                 serdev_controller_write_wakeup(ctrl);
> > +
> > +out:
> > +       wake_up_interruptible_poll(&port->tty->write_wait, POLLOUT);
> 
> Shouldn't this be separate patch?

That change is needed for tty_wait_until_sent to work correctly, which
starts with a wait_event_interruptible_timeout() on write_wait.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 07/10] serdev: add serdev_device_wait_until_sent

[Rob Herring]

On Tue, Mar 7, 2017 at 9:55 AM, Sebastian Reichel <sre@kernel.org> wrote:
> Hi,
>
> On Tue, Mar 07, 2017 at 09:34:49AM -0600, Rob Herring wrote:
>> On Sat, Mar 4, 2017 at 5:58 AM, Sebastian Reichel <sre@kernel.org> wrote:
>> > Add method, which waits until the transmission buffer has been sent.
>> >
>> > Signed-off-by: Sebastian Reichel <sre@kernel.org>
>> > ---
>> >  drivers/tty/serdev/core.c           | 11 +++++++++++
>> >  drivers/tty/serdev/serdev-ttyport.c | 15 ++++++++++++++-
>> >  include/linux/serdev.h              |  3 +++
>> >  3 files changed, 28 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
>> > index f4c6c90add78..a63b74031e22 100644
>> > --- a/drivers/tty/serdev/core.c
>> > +++ b/drivers/tty/serdev/core.c
>> > @@ -173,6 +173,17 @@ void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable)
>> >  }
>> >  EXPORT_SYMBOL_GPL(serdev_device_set_flow_control);
>> >
>> > +void serdev_device_wait_until_sent(struct serdev_device *serdev, long timeout)
>> > +{
>> > +       struct serdev_controller *ctrl = serdev->ctrl;
>> > +
>> > +       if (!ctrl || !ctrl->ops->wait_until_sent)
>> > +               return;
>> > +
>> > +       ctrl->ops->wait_until_sent(ctrl, timeout);
>> > +}
>> > +EXPORT_SYMBOL_GPL(serdev_device_wait_until_sent);
>> > +
>> >  static int serdev_drv_probe(struct device *dev)
>> >  {
>> >         const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
>> > diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
>> > index d05393594f15..db2bc601e554 100644
>> > --- a/drivers/tty/serdev/serdev-ttyport.c
>> > +++ b/drivers/tty/serdev/serdev-ttyport.c
>> > @@ -14,6 +14,7 @@
>> >  #include <linux/serdev.h>
>> >  #include <linux/tty.h>
>> >  #include <linux/tty_driver.h>
>> > +#include <linux/poll.h>
>> >
>> >  #define SERPORT_ACTIVE         1
>> >
>> > @@ -47,10 +48,13 @@ static void ttyport_write_wakeup(struct tty_port *port)
>> >         struct serport *serport = serdev_controller_get_drvdata(ctrl);
>> >
>> >         if (!test_and_clear_bit(TTY_DO_WRITE_WAKEUP, &port->tty->flags))
>> > -               return;
>> > +               goto out;
>> >
>> >         if (test_bit(SERPORT_ACTIVE, &serport->flags))
>> >                 serdev_controller_write_wakeup(ctrl);
>> > +
>> > +out:
>> > +       wake_up_interruptible_poll(&port->tty->write_wait, POLLOUT);
>>
>> Shouldn't this be separate patch?
>
> That change is needed for tty_wait_until_sent to work correctly, which
> starts with a wait_event_interruptible_timeout() on write_wait.

Okay.

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 07/10] serdev: add serdev_device_wait_until_sent

[Rob Herring]

On Tue, Mar 7, 2017 at 9:55 AM, Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> Hi,
>
> On Tue, Mar 07, 2017 at 09:34:49AM -0600, Rob Herring wrote:
>> On Sat, Mar 4, 2017 at 5:58 AM, Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> > Add method, which waits until the transmission buffer has been sent.
>> >
>> > Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> > ---
>> >  drivers/tty/serdev/core.c           | 11 +++++++++++
>> >  drivers/tty/serdev/serdev-ttyport.c | 15 ++++++++++++++-
>> >  include/linux/serdev.h              |  3 +++
>> >  3 files changed, 28 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
>> > index f4c6c90add78..a63b74031e22 100644
>> > --- a/drivers/tty/serdev/core.c
>> > +++ b/drivers/tty/serdev/core.c
>> > @@ -173,6 +173,17 @@ void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable)
>> >  }
>> >  EXPORT_SYMBOL_GPL(serdev_device_set_flow_control);
>> >
>> > +void serdev_device_wait_until_sent(struct serdev_device *serdev, long timeout)
>> > +{
>> > +       struct serdev_controller *ctrl = serdev->ctrl;
>> > +
>> > +       if (!ctrl || !ctrl->ops->wait_until_sent)
>> > +               return;
>> > +
>> > +       ctrl->ops->wait_until_sent(ctrl, timeout);
>> > +}
>> > +EXPORT_SYMBOL_GPL(serdev_device_wait_until_sent);
>> > +
>> >  static int serdev_drv_probe(struct device *dev)
>> >  {
>> >         const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
>> > diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
>> > index d05393594f15..db2bc601e554 100644
>> > --- a/drivers/tty/serdev/serdev-ttyport.c
>> > +++ b/drivers/tty/serdev/serdev-ttyport.c
>> > @@ -14,6 +14,7 @@
>> >  #include <linux/serdev.h>
>> >  #include <linux/tty.h>
>> >  #include <linux/tty_driver.h>
>> > +#include <linux/poll.h>
>> >
>> >  #define SERPORT_ACTIVE         1
>> >
>> > @@ -47,10 +48,13 @@ static void ttyport_write_wakeup(struct tty_port *port)
>> >         struct serport *serport = serdev_controller_get_drvdata(ctrl);
>> >
>> >         if (!test_and_clear_bit(TTY_DO_WRITE_WAKEUP, &port->tty->flags))
>> > -               return;
>> > +               goto out;
>> >
>> >         if (test_bit(SERPORT_ACTIVE, &serport->flags))
>> >                 serdev_controller_write_wakeup(ctrl);
>> > +
>> > +out:
>> > +       wake_up_interruptible_poll(&port->tty->write_wait, POLLOUT);
>>
>> Shouldn't this be separate patch?
>
> That change is needed for tty_wait_until_sent to work correctly, which
> starts with a wait_event_interruptible_timeout() on write_wait.

Okay.

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 08/10] serdev: add serdev_device_get_cts

[Sebastian Reichel]

Add method to get current CTS line state.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/tty/serdev/core.c           | 11 +++++++++++
 drivers/tty/serdev/serdev-ttyport.c | 15 +++++++++++++++
 include/linux/serdev.h              |  6 ++++++
 3 files changed, 32 insertions(+)

diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index a63b74031e22..36eb3dfb1477 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -184,6 +184,17 @@ void serdev_device_wait_until_sent(struct serdev_device *serdev, long timeout)
 }
 EXPORT_SYMBOL_GPL(serdev_device_wait_until_sent);
 
+bool serdev_device_get_cts(struct serdev_device *serdev)
+{
+	struct serdev_controller *ctrl = serdev->ctrl;
+
+	if (!ctrl || !ctrl->ops->get_cts)
+		return false;
+
+	return ctrl->ops->get_cts(ctrl);
+}
+EXPORT_SYMBOL_GPL(serdev_device_get_cts);
+
 static int serdev_drv_probe(struct device *dev)
 {
 	const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
index db2bc601e554..3eaca7560e2e 100644
--- a/drivers/tty/serdev/serdev-ttyport.c
+++ b/drivers/tty/serdev/serdev-ttyport.c
@@ -179,6 +179,20 @@ static void ttyport_wait_until_sent(struct serdev_controller *ctrl, long timeout
 	tty_wait_until_sent(tty, timeout);
 }
 
+static bool ttyport_get_cts(struct serdev_controller *ctrl)
+{
+	struct serport *serport = serdev_controller_get_drvdata(ctrl);
+	struct tty_struct *tty = serport->tty;
+	int status;
+
+	if (!tty->ops->tiocmget)
+		return false;
+
+	status = tty->ops->tiocmget(tty);
+
+	return !!(status & TIOCM_CTS);
+}
+
 static const struct serdev_controller_ops ctrl_ops = {
 	.write_buf = ttyport_write_buf,
 	.write_flush = ttyport_write_flush,
@@ -188,6 +202,7 @@ static const struct serdev_controller_ops ctrl_ops = {
 	.set_flow_control = ttyport_set_flow_control,
 	.set_baudrate = ttyport_set_baudrate,
 	.wait_until_sent = ttyport_wait_until_sent,
+	.get_cts = ttyport_get_cts,
 };
 
 struct device *serdev_tty_port_register(struct tty_port *port,
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index a308b206d204..b0c47402d84a 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -82,6 +82,7 @@ struct serdev_controller_ops {
 	void (*set_flow_control)(struct serdev_controller *, bool);
 	unsigned int (*set_baudrate)(struct serdev_controller *, unsigned int);
 	void (*wait_until_sent)(struct serdev_controller *, long);
+	bool (*get_cts)(struct serdev_controller *);
 };
 
 /**
@@ -188,6 +189,7 @@ void serdev_device_close(struct serdev_device *);
 unsigned int serdev_device_set_baudrate(struct serdev_device *, unsigned int);
 void serdev_device_set_flow_control(struct serdev_device *, bool);
 void serdev_device_wait_until_sent(struct serdev_device *, long);
+bool serdev_device_get_cts(struct serdev_device *);
 int serdev_device_write_buf(struct serdev_device *, const unsigned char *, size_t);
 void serdev_device_write_flush(struct serdev_device *);
 int serdev_device_write_room(struct serdev_device *);
@@ -226,6 +228,10 @@ static inline unsigned int serdev_device_set_baudrate(struct serdev_device *sdev
 }
 static inline void serdev_device_set_flow_control(struct serdev_device *sdev, bool enable) {}
 static inline void serdev_device_wait_until_sent(struct serdev_device *sdev, long timeout) {}
+static inline bool serdev_device_get_cts(struct serdev_device *serdev)
+{
+	return false;
+}
 static inline int serdev_device_write_buf(struct serdev_device *sdev, const unsigned char *buf, size_t count)
 {
 	return -ENODEV;
-- 
2.11.0

Re: [PATCH 08/10] serdev: add serdev_device_get_cts

[Rob Herring]

On Sat, Mar 4, 2017 at 5:58 AM, Sebastian Reichel <sre@kernel.org> wrote:
> Add method to get current CTS line state.
>
> Signed-off-by: Sebastian Reichel <sre@kernel.org>
> ---
>  drivers/tty/serdev/core.c           | 11 +++++++++++
>  drivers/tty/serdev/serdev-ttyport.c | 15 +++++++++++++++
>  include/linux/serdev.h              |  6 ++++++
>  3 files changed, 32 insertions(+)
>
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index a63b74031e22..36eb3dfb1477 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -184,6 +184,17 @@ void serdev_device_wait_until_sent(struct serdev_device *serdev, long timeout)
>  }
>  EXPORT_SYMBOL_GPL(serdev_device_wait_until_sent);
>
> +bool serdev_device_get_cts(struct serdev_device *serdev)

I wonder if this should just be the wait for cts in the core. I think
that would be the common use for reading the CTS state. IIRC, I saw
some other device needing the same thing.

Rob

Re: [PATCH 08/10] serdev: add serdev_device_get_cts

[Rob Herring]

On Sat, Mar 4, 2017 at 5:58 AM, Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> Add method to get current CTS line state.
>
> Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>  drivers/tty/serdev/core.c           | 11 +++++++++++
>  drivers/tty/serdev/serdev-ttyport.c | 15 +++++++++++++++
>  include/linux/serdev.h              |  6 ++++++
>  3 files changed, 32 insertions(+)
>
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index a63b74031e22..36eb3dfb1477 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -184,6 +184,17 @@ void serdev_device_wait_until_sent(struct serdev_device *serdev, long timeout)
>  }
>  EXPORT_SYMBOL_GPL(serdev_device_wait_until_sent);
>
> +bool serdev_device_get_cts(struct serdev_device *serdev)

I wonder if this should just be the wait for cts in the core. I think
that would be the common use for reading the CTS state. IIRC, I saw
some other device needing the same thing.

Rob

Re: [PATCH 08/10] serdev: add serdev_device_get_cts

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1363 bytes --]

Hi Rob,

On Tue, Mar 07, 2017 at 10:03:37AM -0600, Rob Herring wrote:
> On Sat, Mar 4, 2017 at 5:58 AM, Sebastian Reichel <sre@kernel.org> wrote:
> > Add method to get current CTS line state.
> >
> > Signed-off-by: Sebastian Reichel <sre@kernel.org>
> > ---
> >  drivers/tty/serdev/core.c           | 11 +++++++++++
> >  drivers/tty/serdev/serdev-ttyport.c | 15 +++++++++++++++
> >  include/linux/serdev.h              |  6 ++++++
> >  3 files changed, 32 insertions(+)
> >
> > diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> > index a63b74031e22..36eb3dfb1477 100644
> > --- a/drivers/tty/serdev/core.c
> > +++ b/drivers/tty/serdev/core.c
> > @@ -184,6 +184,17 @@ void serdev_device_wait_until_sent(struct serdev_device *serdev, long timeout)
> >  }
> >  EXPORT_SYMBOL_GPL(serdev_device_wait_until_sent);
> >
> > +bool serdev_device_get_cts(struct serdev_device *serdev)
> 
> I wonder if this should just be the wait for cts in the core. I think
> that would be the common use for reading the CTS state. IIRC, I saw
> some other device needing the same thing.

mh. I think it fits nicely to serdev_device_set_rts(). But I guess
it makes sense to have the wait feature directly in serdev. So I
suggest to add the function in addition to the simple get_cts
function. Does that make sense to you?

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 08/10] serdev: add serdev_device_get_cts

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1421 bytes --]

Hi Rob,

On Tue, Mar 07, 2017 at 10:03:37AM -0600, Rob Herring wrote:
> On Sat, Mar 4, 2017 at 5:58 AM, Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > Add method to get current CTS line state.
> >
> > Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > ---
> >  drivers/tty/serdev/core.c           | 11 +++++++++++
> >  drivers/tty/serdev/serdev-ttyport.c | 15 +++++++++++++++
> >  include/linux/serdev.h              |  6 ++++++
> >  3 files changed, 32 insertions(+)
> >
> > diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> > index a63b74031e22..36eb3dfb1477 100644
> > --- a/drivers/tty/serdev/core.c
> > +++ b/drivers/tty/serdev/core.c
> > @@ -184,6 +184,17 @@ void serdev_device_wait_until_sent(struct serdev_device *serdev, long timeout)
> >  }
> >  EXPORT_SYMBOL_GPL(serdev_device_wait_until_sent);
> >
> > +bool serdev_device_get_cts(struct serdev_device *serdev)
> 
> I wonder if this should just be the wait for cts in the core. I think
> that would be the common use for reading the CTS state. IIRC, I saw
> some other device needing the same thing.

mh. I think it fits nicely to serdev_device_set_rts(). But I guess
it makes sense to have the wait feature directly in serdev. So I
suggest to add the function in addition to the simple get_cts
function. Does that make sense to you?

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 08/10] serdev: add serdev_device_get_cts

[Rob Herring]

On Tue, Mar 7, 2017 at 3:12 PM, Sebastian Reichel <sre@kernel.org> wrote:
> Hi Rob,
>
> On Tue, Mar 07, 2017 at 10:03:37AM -0600, Rob Herring wrote:
>> On Sat, Mar 4, 2017 at 5:58 AM, Sebastian Reichel <sre@kernel.org> wrote:
>> > Add method to get current CTS line state.
>> >
>> > Signed-off-by: Sebastian Reichel <sre@kernel.org>
>> > ---
>> >  drivers/tty/serdev/core.c           | 11 +++++++++++
>> >  drivers/tty/serdev/serdev-ttyport.c | 15 +++++++++++++++
>> >  include/linux/serdev.h              |  6 ++++++
>> >  3 files changed, 32 insertions(+)
>> >
>> > diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
>> > index a63b74031e22..36eb3dfb1477 100644
>> > --- a/drivers/tty/serdev/core.c
>> > +++ b/drivers/tty/serdev/core.c
>> > @@ -184,6 +184,17 @@ void serdev_device_wait_until_sent(struct serdev_device *serdev, long timeout)
>> >  }
>> >  EXPORT_SYMBOL_GPL(serdev_device_wait_until_sent);
>> >
>> > +bool serdev_device_get_cts(struct serdev_device *serdev)
>>
>> I wonder if this should just be the wait for cts in the core. I think
>> that would be the common use for reading the CTS state. IIRC, I saw
>> some other device needing the same thing.
>
> mh. I think it fits nicely to serdev_device_set_rts(). But I guess
> it makes sense to have the wait feature directly in serdev. So I
> suggest to add the function in addition to the simple get_cts
> function. Does that make sense to you?

Sure. You should be able to implement that just in the core. Then the
host driver stays simple and we're not duplicating the wait in host
drivers (not that we expect to have lots of host drivers).

Rob

[PATCH 09/10] serdev: add serdev_device_set_rts

[Sebastian Reichel]

Add function to enable/disable RTS line.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/tty/serdev/core.c           | 11 +++++++++++
 drivers/tty/serdev/serdev-ttyport.c | 32 ++++++++++++++++++++++++++++++++
 include/linux/serdev.h              |  3 +++
 3 files changed, 46 insertions(+)

diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index 36eb3dfb1477..f08fe257dadb 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -195,6 +195,17 @@ bool serdev_device_get_cts(struct serdev_device *serdev)
 }
 EXPORT_SYMBOL_GPL(serdev_device_get_cts);
 
+void serdev_device_set_rts(struct serdev_device *serdev, bool enable)
+{
+	struct serdev_controller *ctrl = serdev->ctrl;
+
+	if (!ctrl || !ctrl->ops->set_rts)
+		return;
+
+	return ctrl->ops->set_rts(ctrl, enable);
+}
+EXPORT_SYMBOL_GPL(serdev_device_set_rts);
+
 static int serdev_drv_probe(struct device *dev)
 {
 	const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
index 3eaca7560e2e..06a7d9131181 100644
--- a/drivers/tty/serdev/serdev-ttyport.c
+++ b/drivers/tty/serdev/serdev-ttyport.c
@@ -193,6 +193,37 @@ static bool ttyport_get_cts(struct serdev_controller *ctrl)
 	return !!(status & TIOCM_CTS);
 }
 
+static void ttyport_set_rts(struct serdev_controller *ctrl, bool enable)
+{
+	struct serport *serport = serdev_controller_get_drvdata(ctrl);
+	struct tty_struct *tty = serport->tty;
+	int status = tty->driver->ops->tiocmget(tty);
+	unsigned int set = 0;
+	unsigned int clear = 0;
+
+	if (enable) {
+		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);
+	} else {
+		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)
+		dev_err(&ctrl->dev, "failed to %s RTS: %d",
+			enable ? "enable" : "disable", status);
+}
+
 static const struct serdev_controller_ops ctrl_ops = {
 	.write_buf = ttyport_write_buf,
 	.write_flush = ttyport_write_flush,
@@ -203,6 +234,7 @@ static const struct serdev_controller_ops ctrl_ops = {
 	.set_baudrate = ttyport_set_baudrate,
 	.wait_until_sent = ttyport_wait_until_sent,
 	.get_cts = ttyport_get_cts,
+	.set_rts = ttyport_set_rts,
 };
 
 struct device *serdev_tty_port_register(struct tty_port *port,
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index b0c47402d84a..f520565f9a61 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -83,6 +83,7 @@ struct serdev_controller_ops {
 	unsigned int (*set_baudrate)(struct serdev_controller *, unsigned int);
 	void (*wait_until_sent)(struct serdev_controller *, long);
 	bool (*get_cts)(struct serdev_controller *);
+	void (*set_rts)(struct serdev_controller *, bool);
 };
 
 /**
@@ -190,6 +191,7 @@ unsigned int serdev_device_set_baudrate(struct serdev_device *, unsigned int);
 void serdev_device_set_flow_control(struct serdev_device *, bool);
 void serdev_device_wait_until_sent(struct serdev_device *, long);
 bool serdev_device_get_cts(struct serdev_device *);
+void serdev_device_set_rts(struct serdev_device *, bool);
 int serdev_device_write_buf(struct serdev_device *, const unsigned char *, size_t);
 void serdev_device_write_flush(struct serdev_device *);
 int serdev_device_write_room(struct serdev_device *);
@@ -232,6 +234,7 @@ static inline bool serdev_device_get_cts(struct serdev_device *serdev)
 {
 	return false;
 }
+static inline void serdev_device_set_rts(struct serdev_controller *serdev, bool) {}
 static inline int serdev_device_write_buf(struct serdev_device *sdev, const unsigned char *buf, size_t count)
 {
 	return -ENODEV;
-- 
2.11.0

[PATCH 09/10] serdev: add serdev_device_set_rts

[Sebastian Reichel]

Add function to enable/disable RTS line.

Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/tty/serdev/core.c           | 11 +++++++++++
 drivers/tty/serdev/serdev-ttyport.c | 32 ++++++++++++++++++++++++++++++++
 include/linux/serdev.h              |  3 +++
 3 files changed, 46 insertions(+)

diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index 36eb3dfb1477..f08fe257dadb 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -195,6 +195,17 @@ bool serdev_device_get_cts(struct serdev_device *serdev)
 }
 EXPORT_SYMBOL_GPL(serdev_device_get_cts);
 
+void serdev_device_set_rts(struct serdev_device *serdev, bool enable)
+{
+	struct serdev_controller *ctrl = serdev->ctrl;
+
+	if (!ctrl || !ctrl->ops->set_rts)
+		return;
+
+	return ctrl->ops->set_rts(ctrl, enable);
+}
+EXPORT_SYMBOL_GPL(serdev_device_set_rts);
+
 static int serdev_drv_probe(struct device *dev)
 {
 	const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
index 3eaca7560e2e..06a7d9131181 100644
--- a/drivers/tty/serdev/serdev-ttyport.c
+++ b/drivers/tty/serdev/serdev-ttyport.c
@@ -193,6 +193,37 @@ static bool ttyport_get_cts(struct serdev_controller *ctrl)
 	return !!(status & TIOCM_CTS);
 }
 
+static void ttyport_set_rts(struct serdev_controller *ctrl, bool enable)
+{
+	struct serport *serport = serdev_controller_get_drvdata(ctrl);
+	struct tty_struct *tty = serport->tty;
+	int status = tty->driver->ops->tiocmget(tty);
+	unsigned int set = 0;
+	unsigned int clear = 0;
+
+	if (enable) {
+		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);
+	} else {
+		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)
+		dev_err(&ctrl->dev, "failed to %s RTS: %d",
+			enable ? "enable" : "disable", status);
+}
+
 static const struct serdev_controller_ops ctrl_ops = {
 	.write_buf = ttyport_write_buf,
 	.write_flush = ttyport_write_flush,
@@ -203,6 +234,7 @@ static const struct serdev_controller_ops ctrl_ops = {
 	.set_baudrate = ttyport_set_baudrate,
 	.wait_until_sent = ttyport_wait_until_sent,
 	.get_cts = ttyport_get_cts,
+	.set_rts = ttyport_set_rts,
 };
 
 struct device *serdev_tty_port_register(struct tty_port *port,
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index b0c47402d84a..f520565f9a61 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -83,6 +83,7 @@ struct serdev_controller_ops {
 	unsigned int (*set_baudrate)(struct serdev_controller *, unsigned int);
 	void (*wait_until_sent)(struct serdev_controller *, long);
 	bool (*get_cts)(struct serdev_controller *);
+	void (*set_rts)(struct serdev_controller *, bool);
 };
 
 /**
@@ -190,6 +191,7 @@ unsigned int serdev_device_set_baudrate(struct serdev_device *, unsigned int);
 void serdev_device_set_flow_control(struct serdev_device *, bool);
 void serdev_device_wait_until_sent(struct serdev_device *, long);
 bool serdev_device_get_cts(struct serdev_device *);
+void serdev_device_set_rts(struct serdev_device *, bool);
 int serdev_device_write_buf(struct serdev_device *, const unsigned char *, size_t);
 void serdev_device_write_flush(struct serdev_device *);
 int serdev_device_write_room(struct serdev_device *);
@@ -232,6 +234,7 @@ static inline bool serdev_device_get_cts(struct serdev_device *serdev)
 {
 	return false;
 }
+static inline void serdev_device_set_rts(struct serdev_controller *serdev, bool) {}
 static inline int serdev_device_write_buf(struct serdev_device *sdev, const unsigned char *buf, size_t count)
 {
 	return -ENODEV;
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/10] serdev: add serdev_device_set_rts

[Rob Herring]

On Sat, Mar 4, 2017 at 5:58 AM, Sebastian Reichel <sre@kernel.org> wrote:
> Add function to enable/disable RTS line.
>
> Signed-off-by: Sebastian Reichel <sre@kernel.org>
> ---

> +static void ttyport_set_rts(struct serdev_controller *ctrl, bool enable)
> +{
> +       struct serport *serport = serdev_controller_get_drvdata(ctrl);
> +       struct tty_struct *tty = serport->tty;
> +       int status = tty->driver->ops->tiocmget(tty);
> +       unsigned int set = 0;
> +       unsigned int clear = 0;
> +
> +       if (enable) {
> +               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);
> +       } else {
> +               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);

The logic here can be greatly simplified. Here's a patch to the
original version that I came up with, but haven't tested:

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index de20a438a78a..0e35663c5d04 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -264,8 +264,8 @@ void hci_uart_set_flow_control(struct hci_uart
*hu, bool enable)
        struct tty_struct *tty = hu->tty;
        struct ktermios ktermios;
        int status;
-       unsigned int set = 0;
-       unsigned int clear = 0;
+       unsigned int set;
+       unsigned int clear = TIOCM_DTR | TIOCM_OUT1 | TIOCM_LOOP;

        if (enable) {
                /* Disable hardware flow control */
@@ -280,25 +280,15 @@ void hci_uart_set_flow_control(struct hci_uart
*hu, bool enable)
                status = tty->driver->ops->tiocmget(tty);
                BT_DBG("Current tiocm 0x%x", 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);
+               clear |= TIOCM_RTS | TIOCM_OUT2;
+               status = tty->driver->ops->tiocmset(tty, 0, clear);
                BT_DBG("Clearing RTS: %s", status ? "failed" : "success");
        } else {
                /* Set RTS to allow the device to send again */
                status = tty->driver->ops->tiocmget(tty);
                BT_DBG("Current tiocm 0x%x", 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;
+               set = TIOCM_OUT2 | TIOCM_RTS;
                status = tty->driver->ops->tiocmset(tty, set, clear);
                BT_DBG("Setting RTS: %s", status ? "failed" : "success");

Re: [PATCH 09/10] serdev: add serdev_device_set_rts

[Rob Herring]

On Sat, Mar 4, 2017 at 5:58 AM, Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> Add function to enable/disable RTS line.
>
> Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---

> +static void ttyport_set_rts(struct serdev_controller *ctrl, bool enable)
> +{
> +       struct serport *serport = serdev_controller_get_drvdata(ctrl);
> +       struct tty_struct *tty = serport->tty;
> +       int status = tty->driver->ops->tiocmget(tty);
> +       unsigned int set = 0;
> +       unsigned int clear = 0;
> +
> +       if (enable) {
> +               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);
> +       } else {
> +               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);

The logic here can be greatly simplified. Here's a patch to the
original version that I came up with, but haven't tested:

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index de20a438a78a..0e35663c5d04 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -264,8 +264,8 @@ void hci_uart_set_flow_control(struct hci_uart
*hu, bool enable)
        struct tty_struct *tty = hu->tty;
        struct ktermios ktermios;
        int status;
-       unsigned int set = 0;
-       unsigned int clear = 0;
+       unsigned int set;
+       unsigned int clear = TIOCM_DTR | TIOCM_OUT1 | TIOCM_LOOP;

        if (enable) {
                /* Disable hardware flow control */
@@ -280,25 +280,15 @@ void hci_uart_set_flow_control(struct hci_uart
*hu, bool enable)
                status = tty->driver->ops->tiocmget(tty);
                BT_DBG("Current tiocm 0x%x", 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);
+               clear |= TIOCM_RTS | TIOCM_OUT2;
+               status = tty->driver->ops->tiocmset(tty, 0, clear);
                BT_DBG("Clearing RTS: %s", status ? "failed" : "success");
        } else {
                /* Set RTS to allow the device to send again */
                status = tty->driver->ops->tiocmget(tty);
                BT_DBG("Current tiocm 0x%x", 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;
+               set = TIOCM_OUT2 | TIOCM_RTS;
                status = tty->driver->ops->tiocmset(tty, set, clear);
                BT_DBG("Setting RTS: %s", status ? "failed" : "success");

Re: [PATCH 09/10] serdev: add serdev_device_set_rts

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1802 bytes --]

Hi Rob,

On Tue, Mar 07, 2017 at 10:07:56AM -0600, Rob Herring wrote:
> On Sat, Mar 4, 2017 at 5:58 AM, Sebastian Reichel <sre@kernel.org> wrote:
> > Add function to enable/disable RTS line.
> >
> > Signed-off-by: Sebastian Reichel <sre@kernel.org>
> > ---
> 
> > +static void ttyport_set_rts(struct serdev_controller *ctrl, bool enable)
> > +{
> > +       struct serport *serport = serdev_controller_get_drvdata(ctrl);
> > +       struct tty_struct *tty = serport->tty;
> > +       int status = tty->driver->ops->tiocmget(tty);
> > +       unsigned int set = 0;
> > +       unsigned int clear = 0;
> > +
> > +       if (enable) {
> > +               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);
> > +       } else {
> > +               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);
> 
> The logic here can be greatly simplified. Here's a patch to the
> original version that I came up with, but haven't tested:

ah I totally forgot to clean that mess. I took that over from the
bluetooth code, since that got the nokia bluetooth chips working.
I will do this in the next revision.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 09/10] serdev: add serdev_device_set_rts

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1860 bytes --]

Hi Rob,

On Tue, Mar 07, 2017 at 10:07:56AM -0600, Rob Herring wrote:
> On Sat, Mar 4, 2017 at 5:58 AM, Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > Add function to enable/disable RTS line.
> >
> > Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > ---
> 
> > +static void ttyport_set_rts(struct serdev_controller *ctrl, bool enable)
> > +{
> > +       struct serport *serport = serdev_controller_get_drvdata(ctrl);
> > +       struct tty_struct *tty = serport->tty;
> > +       int status = tty->driver->ops->tiocmget(tty);
> > +       unsigned int set = 0;
> > +       unsigned int clear = 0;
> > +
> > +       if (enable) {
> > +               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);
> > +       } else {
> > +               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);
> 
> The logic here can be greatly simplified. Here's a patch to the
> original version that I came up with, but haven't tested:

ah I totally forgot to clean that mess. I took that over from the
bluetooth code, since that got the nokia bluetooth chips working.
I will do this in the next revision.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 10/10] Bluetooth: add nokia driver

[Sebastian Reichel]

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 ++
 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"
+ - reset-gpios: GPIO specifier, used to reset the BT module
+ - bluetooth-wakeup-gpios: GPIO specifier, used to wakeup the BT module
+ - host-wakeup-gpios: GPIO specifier, used to wakeup the host processor
+ - 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";
+       };
+};
diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index c2c14a12713b..2e3e4d3547ad 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -86,6 +86,18 @@ config BT_HCIUART_H4
 
 	  Say Y here to compile support for HCI UART (H4) protocol.
 
+config BT_HCIUART_NOKIA
+	tristate "UART Nokia H4+ protocol support"
+	depends on BT_HCIUART
+	depends on SERIAL_DEV_BUS
+	depends on PM
+	help
+	  Nokia H4+ is serial protocol for communication between Bluetooth
+	  device and host. This protocol is required for Bluetooth devices
+	  with UART interface in Nokia devices.
+
+	  Say Y here to compile support for Nokia's H4+ protocol.
+
 config BT_HCIUART_BCSP
 	bool "BCSP protocol support"
 	depends on BT_HCIUART
diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
index fd571689eed6..a7f237320f4b 100644
--- a/drivers/bluetooth/Makefile
+++ b/drivers/bluetooth/Makefile
@@ -25,6 +25,8 @@ obj-$(CONFIG_BT_BCM)		+= btbcm.o
 obj-$(CONFIG_BT_RTL)		+= btrtl.o
 obj-$(CONFIG_BT_QCA)		+= btqca.o
 
+obj-$(CONFIG_BT_HCIUART_NOKIA)	+= hci_nokia.o
+
 btmrvl-y			:= btmrvl_main.o
 btmrvl-$(CONFIG_DEBUG_FS)	+= btmrvl_debugfs.o
 
diff --git a/drivers/bluetooth/hci_nokia.c b/drivers/bluetooth/hci_nokia.c
new file mode 100644
index 000000000000..8dc397a47065
--- /dev/null
+++ b/drivers/bluetooth/hci_nokia.c
@@ -0,0 +1,839 @@
+/*
+ *  Bluetooth HCI UART H4 driver with Nokia Extensions AKA Nokia H4+
+ *
+ *  Copyright (C) 2015 Marcel Holtmann <marcel@holtmann.org>
+ *  Copyright (C) 2015-2017 Sebastian Reichel <sre@kernel.org>
+ *
+ *  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/clk.h>
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/interrupt.h>
+#include <linux/pm_runtime.h>
+#include <linux/firmware.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/errno.h>
+#include <linux/skbuff.h>
+#include <linux/gpio/consumer.h>
+#include <linux/unaligned/le_struct.h>
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci_core.h>
+#include <linux/serdev.h>
+
+#include "hci_uart.h"
+#include "btbcm.h"
+
+#define NOKIA_ID_BCM2048	0x04
+#define NOKIA_ID_TI1271		0x31
+
+#define FIRMWARE_BCM2048	"nokia/bcmfw.bin"
+#define FIRMWARE_TI1271		"nokia/ti1273.bin"
+
+#define HCI_NOKIA_NEG_PKT	0x06
+#define HCI_NOKIA_ALIVE_PKT	0x07
+#define HCI_NOKIA_RADIO_PKT	0x08
+
+#define HCI_NOKIA_NEG_HDR_SIZE		1
+#define HCI_NOKIA_MAX_NEG_SIZE		255
+#define HCI_NOKIA_ALIVE_HDR_SIZE	1
+#define HCI_NOKIA_MAX_ALIVE_SIZE	255
+#define HCI_NOKIA_RADIO_HDR_SIZE	2
+#define HCI_NOKIA_MAX_RADIO_SIZE	255
+
+#define NOKIA_PROTO_PKT		0x44
+#define NOKIA_PROTO_BYTE	0x4c
+
+#define NOKIA_NEG_REQ		0x00
+#define NOKIA_NEG_ACK		0x20
+#define NOKIA_NEG_NAK		0x40
+
+#define H4_TYPE_SIZE		1
+
+#define NOKIA_RECV_ALIVE \
+	.type = HCI_NOKIA_ALIVE_PKT, \
+	.hlen = HCI_NOKIA_ALIVE_HDR_SIZE, \
+	.loff = 0, \
+	.lsize = 1, \
+	.maxlen = HCI_NOKIA_MAX_ALIVE_SIZE \
+
+#define NOKIA_RECV_NEG \
+	.type = HCI_NOKIA_NEG_PKT, \
+	.hlen = HCI_NOKIA_NEG_HDR_SIZE, \
+	.loff = 0, \
+	.lsize = 1, \
+	.maxlen = HCI_NOKIA_MAX_NEG_SIZE \
+
+#define NOKIA_RECV_RADIO \
+	.type = HCI_NOKIA_RADIO_PKT, \
+	.hlen = HCI_NOKIA_RADIO_HDR_SIZE, \
+	.loff = 1, \
+	.lsize = 1, \
+	.maxlen = HCI_NOKIA_MAX_RADIO_SIZE \
+
+struct hci_nokia_neg_hdr {
+	__u8	dlen;
+} __packed;
+
+struct hci_nokia_neg_cmd {
+	__u8	ack;
+	__u16	baud;
+	__u16	unused1;
+	__u8	proto;
+	__u16	sys_clk;
+	__u16	unused2;
+} __packed;
+
+#define NOKIA_ALIVE_REQ   0x55
+#define NOKIA_ALIVE_RESP  0xcc
+
+struct hci_nokia_alive_hdr {
+	__u8	dlen;
+} __packed;
+
+struct hci_nokia_alive_pkt {
+	__u8	mid;
+	__u8	unused;
+} __packed;
+
+struct hci_nokia_neg_evt {
+	__u8	ack;
+	__u16	baud;
+	__u16	unused1;
+	__u8	proto;
+	__u16	sys_clk;
+	__u16	unused2;
+	__u8	man_id;
+	__u8	ver_id;
+} __packed;
+
+#define BT_BAUDRATE_DIVIDER	384000000
+#define MAX_BAUD_RATE		3692300
+#define SETUP_BAUD_RATE		921600
+#define INIT_BAUD_RATE		120000
+
+struct hci_nokia_radio_hdr {
+	__u8	evt;
+	__u8	dlen;
+} __packed;
+
+struct nokia_bt_dev {
+	struct hci_uart hu;
+	struct serdev_device *serdev;
+
+	struct gpio_desc *reset;
+	struct gpio_desc *wakeup_host;
+	struct gpio_desc *wakeup_bt;
+	unsigned long sysclk_speed;
+
+	int wake_irq;
+	struct sk_buff *rx_skb;
+	struct sk_buff_head txq;
+	bdaddr_t bdaddr;
+
+	int init_error;
+	struct completion init_completion;
+
+	uint8_t man_id;
+	uint8_t ver_id;
+
+	bool initialized;
+	bool tx_enabled;
+	bool rx_enabled;
+};
+
+static int nokia_enqueue(struct hci_uart *hu, struct sk_buff *skb);
+
+static int nokia_wait_for_cts(struct hci_uart *hu, bool state,
+				 int timeout_ms)
+{
+	struct nokia_bt_dev *btdev = hu->priv;
+	struct device *dev = &btdev->serdev->dev;
+	unsigned long timeout;
+	bool signal;
+
+	timeout = jiffies + msecs_to_jiffies(timeout_ms);
+	while (!time_after(jiffies, timeout)) {
+		signal = serdev_device_get_cts(btdev->serdev);
+		if (signal == state) {
+			dev_dbg(dev, "wait for cts... received!");
+			return 0;
+		}
+		usleep_range(1000, 2000);
+	}
+
+	return -ETIMEDOUT;
+}
+
+static void nokia_flow_control(struct serdev_device *serdev, bool enable)
+{
+	if (enable) {
+		serdev_device_set_rts(serdev, true);
+		serdev_device_set_flow_control(serdev, true);
+	} else {
+		serdev_device_set_flow_control(serdev, false);
+		serdev_device_set_rts(serdev, false);
+	}
+}
+
+static irqreturn_t wakeup_handler(int irq, void *data)
+{
+	struct nokia_bt_dev *btdev = data;
+	struct device *dev = &btdev->serdev->dev;
+	int wake_state = gpiod_get_value(btdev->wakeup_host);
+
+	if (btdev->rx_enabled == wake_state)
+		return IRQ_HANDLED;
+
+	if (wake_state)
+		pm_runtime_get(dev);
+	else
+		pm_runtime_put(dev);
+
+	btdev->rx_enabled = wake_state;
+
+	return IRQ_HANDLED;
+}
+
+static int nokia_reset(struct hci_uart *hu)
+{
+	struct nokia_bt_dev *btdev = hu->priv;
+	struct device *dev = &btdev->serdev->dev;
+	int err;
+
+	/* reset routine */
+	gpiod_set_value_cansleep(btdev->reset, 1);
+	gpiod_set_value_cansleep(btdev->wakeup_bt, 1);
+
+	msleep(100);
+
+	/* safety check */
+	err = gpiod_get_value_cansleep(btdev->wakeup_host);
+	if (err == 1) {
+		dev_err(dev, "reset: host wakeup not low!");
+		return -EPROTO;
+	}
+
+	/* flush queue */
+	serdev_device_write_flush(btdev->serdev);
+
+	/* init uart */
+	nokia_flow_control(btdev->serdev, false);
+	serdev_device_set_baudrate(btdev->serdev, INIT_BAUD_RATE);
+
+	gpiod_set_value_cansleep(btdev->reset, 0);
+
+	/* wait for cts */
+	err = nokia_wait_for_cts(hu, true, 200);
+	if (err < 0) {
+		dev_err(dev, "CTS not received: %d", err);
+		return err;
+	}
+
+	nokia_flow_control(btdev->serdev, true);
+
+	return 0;
+}
+
+static int nokia_send_alive_packet(struct hci_uart *hu)
+{
+	struct nokia_bt_dev *btdev = hu->priv;
+	struct device *dev = &btdev->serdev->dev;
+	struct hci_nokia_alive_hdr *hdr;
+	struct hci_nokia_alive_pkt *pkt;
+	struct sk_buff *skb;
+	int len;
+
+	init_completion(&btdev->init_completion);
+
+	len = H4_TYPE_SIZE + sizeof(*hdr) + sizeof(*pkt);
+	skb = bt_skb_alloc(len, GFP_KERNEL);
+	if (!skb)
+		return -ENOMEM;
+
+	hci_skb_pkt_type(skb) = HCI_NOKIA_ALIVE_PKT;
+	memset(skb->data, 0x00, len);
+
+	hdr = (struct hci_nokia_alive_hdr *)skb_put(skb, sizeof(*hdr));
+	hdr->dlen = sizeof(*pkt);
+	pkt = (struct hci_nokia_alive_pkt *)skb_put(skb, sizeof(*pkt));
+	pkt->mid = NOKIA_ALIVE_REQ;
+
+	nokia_enqueue(hu, skb);
+	hci_uart_tx_wakeup(hu);
+
+	dev_dbg(dev, "Alive sent");
+
+	if (!wait_for_completion_interruptible_timeout(&btdev->init_completion,
+		msecs_to_jiffies(1000))) {
+		return -ETIMEDOUT;
+	}
+
+	if (btdev->init_error < 0)
+		return btdev->init_error;
+
+	return 0;
+}
+
+static int nokia_send_negotiation(struct hci_uart *hu)
+{
+	struct nokia_bt_dev *btdev = hu->priv;
+	struct device *dev = &btdev->serdev->dev;
+	struct hci_nokia_neg_cmd *neg_cmd;
+	struct hci_nokia_neg_hdr *neg_hdr;
+	struct sk_buff *skb;
+	int len, err;
+	u16 baud = DIV_ROUND_CLOSEST(BT_BAUDRATE_DIVIDER, SETUP_BAUD_RATE);
+	int sysclk = btdev->sysclk_speed / 1000;
+
+	len = H4_TYPE_SIZE + sizeof(*neg_hdr) + sizeof(*neg_cmd);
+	skb = bt_skb_alloc(len, GFP_KERNEL);
+	if (!skb)
+		return -ENOMEM;
+
+	hci_skb_pkt_type(skb) = HCI_NOKIA_NEG_PKT;
+
+	neg_hdr = (struct hci_nokia_neg_hdr *)skb_put(skb, sizeof(*neg_hdr));
+	neg_hdr->dlen = sizeof(*neg_cmd);
+
+	neg_cmd = (struct hci_nokia_neg_cmd *)skb_put(skb, sizeof(*neg_cmd));
+	neg_cmd->ack = NOKIA_NEG_REQ;
+	neg_cmd->baud = cpu_to_le16(baud);
+	neg_cmd->unused1 = 0x0000;
+	neg_cmd->proto = NOKIA_PROTO_BYTE;
+	neg_cmd->sys_clk = cpu_to_le16(sysclk);
+	neg_cmd->unused2 = 0x0000;
+
+	btdev->init_error = 0;
+	init_completion(&btdev->init_completion);
+
+	nokia_enqueue(hu, skb);
+	hci_uart_tx_wakeup(hu);
+
+	dev_dbg(dev, "Negotiation sent");
+
+	if (!wait_for_completion_interruptible_timeout(&btdev->init_completion,
+		msecs_to_jiffies(10000))) {
+		return -ETIMEDOUT;
+	}
+
+	if (btdev->init_error < 0)
+		return btdev->init_error;
+
+	/* Change to previously negotiated speed. Flow Control
+	 * is disabled until bluetooth adapter is ready to avoid
+	 * broken bytes being ready by the bluetooth adapter
+	 */
+	nokia_flow_control(btdev->serdev, false);
+	serdev_device_set_baudrate(btdev->serdev, SETUP_BAUD_RATE);
+	err = nokia_wait_for_cts(hu, true, 200);
+	if (err < 0) {
+		dev_err(dev, "CTS not received: %d", err);
+		return err;
+	}
+	nokia_flow_control(btdev->serdev, true);
+
+	dev_dbg(dev, "Negotiation successful");
+
+	return 0;
+}
+
+static int nokia_setup_fw(struct hci_uart *hu)
+{
+	struct nokia_bt_dev *btdev = hu->priv;
+	struct device *dev = &btdev->serdev->dev;
+	const char *fwname;
+	const struct firmware *fw;
+	const u8 *fw_ptr;
+	size_t fw_size;
+	int err;
+
+	dev_dbg(dev, "setup firmware");
+
+	if (btdev->man_id == NOKIA_ID_BCM2048) {
+		fwname = FIRMWARE_BCM2048;
+	} else if (btdev->man_id == NOKIA_ID_TI1271) {
+		fwname = FIRMWARE_TI1271;
+	} else {
+		dev_err(dev, "Unsupported bluetooth device!");
+		return -ENODEV;
+	}
+
+	err = request_firmware(&fw, fwname, dev);
+	if (err < 0) {
+		dev_err(dev, "%s: Failed to load Nokia firmware file (%d)",
+			hu->hdev->name, err);
+		return err;
+	}
+
+	fw_ptr = fw->data;
+	fw_size = fw->size;
+
+	while (fw_size >= 4) {
+		u16 pkt_size = get_unaligned_le16(fw_ptr);
+		u8 pkt_type = fw_ptr[2];
+		const struct hci_command_hdr *cmd;
+		u16 opcode;
+		struct sk_buff *skb;
+
+		switch (pkt_type) {
+		case HCI_COMMAND_PKT:
+			cmd = (struct hci_command_hdr *)(fw_ptr + 3);
+			opcode = le16_to_cpu(cmd->opcode);
+
+			skb = __hci_cmd_sync(hu->hdev, opcode, cmd->plen,
+					     fw_ptr + 3 + HCI_COMMAND_HDR_SIZE,
+					     HCI_INIT_TIMEOUT);
+			if (IS_ERR(skb)) {
+				err = PTR_ERR(skb);
+				dev_err(dev, "%s: FW command %04x failed (%d)",
+				       hu->hdev->name, opcode, err);
+				goto done;
+			}
+			kfree_skb(skb);
+			break;
+		case HCI_NOKIA_RADIO_PKT:
+		case HCI_NOKIA_NEG_PKT:
+		case HCI_NOKIA_ALIVE_PKT:
+			break;
+		}
+
+		fw_ptr += pkt_size + 2;
+		fw_size -= pkt_size + 2;
+	}
+
+done:
+	release_firmware(fw);
+	return err;
+}
+
+static int nokia_setup(struct hci_uart *hu)
+{
+	struct nokia_bt_dev *btdev = hu->priv;
+	struct device *dev = &btdev->serdev->dev;
+	int err;
+
+	btdev->initialized = false;
+
+	nokia_flow_control(btdev->serdev, false);
+
+	pm_runtime_get_sync(dev);
+
+	if (btdev->tx_enabled) {
+		gpiod_set_value_cansleep(btdev->wakeup_bt, 0);
+		pm_runtime_put(&btdev->serdev->dev);
+		btdev->tx_enabled = false;
+	}
+
+	dev_dbg(dev, "protocol setup");
+
+	/* 0. reset connection */
+	err = nokia_reset(hu);
+	if (err < 0) {
+		dev_err(dev, "Reset failed: %d", err);
+		goto out;
+	}
+
+	/* 1. negotiate speed etc */
+	err = nokia_send_negotiation(hu);
+	if (err < 0) {
+		dev_err(dev, "Negotiation failed: %d", err);
+		goto out;
+	}
+
+	/* 2. verify correct setup using alive packet */
+	err = nokia_send_alive_packet(hu);
+	if (err < 0) {
+		dev_err(dev, "Alive check failed: %d", err);
+		goto out;
+	}
+
+	/* 3. send firmware */
+	err = nokia_setup_fw(hu);
+	if (err < 0) {
+		dev_err(dev, "Could not setup FW: %d", err);
+		goto out;
+	}
+
+	nokia_flow_control(btdev->serdev, false);
+	serdev_device_set_baudrate(btdev->serdev, MAX_BAUD_RATE);
+	nokia_flow_control(btdev->serdev, true);
+
+	if (btdev->man_id == NOKIA_ID_BCM2048) {
+		hu->hdev->set_bdaddr = btbcm_set_bdaddr;
+		set_bit(HCI_QUIRK_INVALID_BDADDR, &hu->hdev->quirks);
+		dev_dbg(dev, "bcm2048 has invalid bluetooth address!");
+	}
+
+	dev_dbg(dev, "protocol setup done!");
+
+	gpiod_set_value_cansleep(btdev->wakeup_bt, 0);
+	pm_runtime_put(dev);
+	btdev->tx_enabled = false;
+	btdev->initialized = true;
+
+	return 0;
+out:
+	pm_runtime_put(dev);
+
+	return err;
+}
+
+static int nokia_open(struct hci_uart *hu)
+{
+	struct device *dev = &hu->serdev->dev;
+
+	dev_dbg(dev, "protocol open");
+
+	serdev_device_open(hu->serdev);
+
+	pm_runtime_enable(dev);
+
+	return 0;
+}
+
+static int nokia_flush(struct hci_uart *hu)
+{
+	struct nokia_bt_dev *btdev = hu->priv;
+
+	dev_dbg(&btdev->serdev->dev, "flush device");
+
+	skb_queue_purge(&btdev->txq);
+
+	return 0;
+}
+
+static int nokia_close(struct hci_uart *hu)
+{
+	struct nokia_bt_dev *btdev = hu->priv;
+	struct device *dev = &btdev->serdev->dev;
+
+	dev_dbg(dev, "close device");
+
+	btdev->initialized = false;
+
+	skb_queue_purge(&btdev->txq);
+
+	kfree_skb(btdev->rx_skb);
+
+	/* disable module */
+	gpiod_set_value(btdev->reset, 1);
+	gpiod_set_value(btdev->wakeup_bt, 0);
+
+	pm_runtime_disable(&btdev->serdev->dev);
+	serdev_device_close(btdev->serdev);
+
+	return 0;
+}
+
+/* Enqueue frame for transmittion (padding, crc, etc) */
+static int nokia_enqueue(struct hci_uart *hu, struct sk_buff *skb)
+{
+	struct nokia_bt_dev *btdev = hu->priv;
+	int err;
+
+	/* Prepend skb with frame type */
+	memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
+
+	/* Packets must be word aligned */
+	if (skb->len % 2) {
+		err = skb_pad(skb, 1);
+		if (err)
+			return err;
+		*skb_put(skb, 1) = 0x00;
+	}
+
+	skb_queue_tail(&btdev->txq, skb);
+
+	return 0;
+}
+
+static int nokia_recv_negotiation_packet(struct hci_dev *hdev,
+					 struct sk_buff *skb)
+{
+	struct hci_uart *hu = hci_get_drvdata(hdev);
+	struct nokia_bt_dev *btdev = hu->priv;
+	struct device *dev = &btdev->serdev->dev;
+	struct hci_nokia_neg_hdr *hdr;
+	struct hci_nokia_neg_evt *evt;
+	int ret = 0;
+
+	hdr = (struct hci_nokia_neg_hdr *)skb->data;
+	if (hdr->dlen != sizeof(*evt)) {
+		btdev->init_error = -EIO;
+		ret = -EIO;
+		goto finish_neg;
+	}
+
+	evt = (struct hci_nokia_neg_evt *)skb_pull(skb, sizeof(*hdr));
+
+	if (evt->ack != NOKIA_NEG_ACK) {
+		dev_err(dev, "Negotiation received: wrong reply");
+		btdev->init_error = -EINVAL;
+	}
+
+	btdev->man_id = evt->man_id;
+	btdev->ver_id = evt->ver_id;
+
+	dev_dbg(dev, "Negotiation received: baud=%u:clk=%u:manu=%u:vers=%u",
+		evt->baud, evt->sys_clk, evt->man_id, evt->ver_id);
+
+finish_neg:
+	complete(&btdev->init_completion);
+	kfree_skb(skb);
+	return ret;
+}
+
+static int nokia_recv_alive_packet(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	struct hci_uart *hu = hci_get_drvdata(hdev);
+	struct nokia_bt_dev *btdev = hu->priv;
+	struct device *dev = &btdev->serdev->dev;
+	struct hci_nokia_alive_hdr *hdr;
+	struct hci_nokia_alive_pkt *pkt;
+	int ret = 0;
+
+	hdr = (struct hci_nokia_alive_hdr *)skb->data;
+	if (hdr->dlen != sizeof(*pkt)) {
+		dev_err(dev, "Corrupted alive message");
+		btdev->init_error = -EIO;
+		ret = -EIO;
+		goto finish_alive;
+	}
+
+	pkt = (struct hci_nokia_alive_pkt *)skb_pull(skb, sizeof(*hdr));
+
+	if (pkt->mid != NOKIA_ALIVE_RESP) {
+		dev_err(dev, "Alive received: invalid response: 0x%02x!",
+			pkt->mid);
+		btdev->init_error = -EINVAL;
+		goto finish_alive;
+	}
+
+	dev_dbg(dev, "Alive received");
+
+finish_alive:
+	complete(&btdev->init_completion);
+	kfree_skb(skb);
+	return ret;
+}
+
+static int nokia_recv_radio(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	/* Packets received on the dedicated radio channel are
+	 * HCI events and so feed them back into the core.
+	 */
+	hci_skb_pkt_type(skb) = HCI_EVENT_PKT;
+	return hci_recv_frame(hdev, skb);
+}
+
+/* Recv data */
+static const struct h4_recv_pkt nokia_recv_pkts[] = {
+	{ H4_RECV_ACL,		.recv = hci_recv_frame },
+	{ H4_RECV_SCO,		.recv = hci_recv_frame },
+	{ H4_RECV_EVENT,	.recv = hci_recv_frame },
+	{ NOKIA_RECV_ALIVE,	.recv = nokia_recv_alive_packet },
+	{ NOKIA_RECV_NEG,	.recv = nokia_recv_negotiation_packet },
+	{ NOKIA_RECV_RADIO,	.recv = nokia_recv_radio },
+};
+
+static int nokia_recv(struct hci_uart *hu, const void *data, int count)
+{
+	struct nokia_bt_dev *btdev = hu->priv;
+	struct device *dev = &btdev->serdev->dev;
+	int err;
+
+	if (!test_bit(HCI_UART_REGISTERED, &hu->flags))
+		return -EUNATCH;
+
+	btdev->rx_skb = h4_recv_buf(hu->hdev, btdev->rx_skb, data, count,
+				  nokia_recv_pkts, ARRAY_SIZE(nokia_recv_pkts));
+	if (IS_ERR(btdev->rx_skb)) {
+		err = PTR_ERR(btdev->rx_skb);
+		dev_err(dev, "Frame reassembly failed (%d)", err);
+		btdev->rx_skb = NULL;
+		return err;
+	}
+
+	return count;
+}
+
+static struct sk_buff *nokia_dequeue(struct hci_uart *hu)
+{
+	struct nokia_bt_dev *btdev = hu->priv;
+	struct device *dev = &btdev->serdev->dev;
+	struct sk_buff *result = skb_dequeue(&btdev->txq);
+
+	if (!btdev->initialized)
+		return result;
+
+	if (btdev->tx_enabled == !!result)
+		return result;
+
+	if (result) {
+		pm_runtime_get_sync(dev);
+		gpiod_set_value_cansleep(btdev->wakeup_bt, 1);
+	} else {
+		serdev_device_wait_until_sent(btdev->serdev, 0);
+		gpiod_set_value_cansleep(btdev->wakeup_bt, 0);
+		pm_runtime_put(dev);
+	}
+
+	btdev->tx_enabled = !!result;
+
+	return result;
+}
+
+static const struct hci_uart_proto nokia_proto = {
+	.id		= HCI_UART_NOKIA,
+	.name		= "Nokia",
+	.open		= nokia_open,
+	.close		= nokia_close,
+	.recv		= nokia_recv,
+	.enqueue	= nokia_enqueue,
+	.dequeue	= nokia_dequeue,
+	.flush		= nokia_flush,
+	.setup		= nokia_setup,
+	.manufacturer	= 1,
+};
+
+static int nokia_bluetooth_serdev_probe(struct serdev_device *serdev)
+{
+	struct device *dev = &serdev->dev;
+	struct nokia_bt_dev *btdev;
+	struct clk *sysclk;
+	int err = 0;
+
+	btdev = devm_kzalloc(dev, sizeof(*btdev), GFP_KERNEL);
+	if (!btdev)
+		return -ENOMEM;
+
+	btdev->hu.serdev = btdev->serdev = serdev;
+	serdev_device_set_drvdata(serdev, btdev);
+
+	btdev->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(btdev->reset)) {
+		err = PTR_ERR(btdev->reset);
+		dev_err(dev, "could not get reset gpio: %d", err);
+		return err;
+	}
+
+	btdev->wakeup_host = devm_gpiod_get(dev, "host-wakeup", GPIOD_IN);
+	if (IS_ERR(btdev->wakeup_host)) {
+		err = PTR_ERR(btdev->wakeup_host);
+		dev_err(dev, "could not get host wakeup gpio: %d", err);
+		return err;
+	}
+
+	btdev->wake_irq = gpiod_to_irq(btdev->wakeup_host);
+
+	err = devm_request_threaded_irq(dev, btdev->wake_irq, NULL,
+		wakeup_handler,
+		IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+		"wakeup", btdev);
+	if (err) {
+		dev_err(dev, "could request wakeup irq: %d", err);
+		return err;
+	}
+
+	btdev->wakeup_bt = devm_gpiod_get(dev, "bluetooth-wakeup",
+					   GPIOD_OUT_LOW);
+	if (IS_ERR(btdev->wakeup_bt)) {
+		err = PTR_ERR(btdev->wakeup_bt);
+		dev_err(dev, "could not get BT wakeup gpio: %d", err);
+		return err;
+	}
+
+	sysclk = devm_clk_get(dev, "sysclk");
+	if (IS_ERR(sysclk)) {
+		err = PTR_ERR(sysclk);
+		dev_err(dev, "could not get sysclk: %d", err);
+		return err;
+	}
+
+	clk_prepare_enable(sysclk);
+	btdev->sysclk_speed = clk_get_rate(sysclk);
+	clk_disable_unprepare(sysclk);
+
+	skb_queue_head_init(&btdev->txq);
+
+	btdev->hu.priv = btdev;
+	btdev->hu.alignment = 2; /* Nokia H4+ is word aligned */
+
+	err = hci_uart_register_device(&btdev->hu, &nokia_proto);
+	if (err) {
+		dev_err(dev, "could not register bluetooth uart: %d", err);
+		return err;
+	}
+
+	return 0;
+}
+
+static void nokia_bluetooth_serdev_remove(struct serdev_device *serdev)
+{
+	struct nokia_bt_dev *btdev = serdev_device_get_drvdata(serdev);
+	struct hci_uart *hu = &btdev->hu;
+	struct hci_dev *hdev = hu->hdev;
+
+	cancel_work_sync(&hu->write_work);
+
+	hci_unregister_dev(hdev);
+	hci_free_dev(hdev);
+	hu->proto->close(hu);
+
+	pm_runtime_disable(&btdev->serdev->dev);
+}
+
+static int nokia_bluetooth_runtime_suspend(struct device *dev)
+{
+	struct serdev_device *serdev = to_serdev_device(dev);
+
+	nokia_flow_control(serdev, false);
+	return 0;
+}
+
+static int nokia_bluetooth_runtime_resume(struct device *dev)
+{
+	struct serdev_device *serdev = to_serdev_device(dev);
+
+	nokia_flow_control(serdev, true);
+	return 0;
+}
+
+static const struct dev_pm_ops nokia_bluetooth_pm_ops = {
+	SET_RUNTIME_PM_OPS(nokia_bluetooth_runtime_suspend,
+			   nokia_bluetooth_runtime_resume,
+			   NULL)
+};
+
+#ifdef CONFIG_OF
+static const struct of_device_id nokia_bluetooth_of_match[] = {
+	{ .compatible = "nokia,brcm,bcm2048", },
+	{ .compatible = "nokia,ti,wl1271-bluetooth", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, nokia_bluetooth_of_match);
+#endif
+
+static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
+	.probe = nokia_bluetooth_serdev_probe,
+	.remove = nokia_bluetooth_serdev_remove,
+	.driver = {
+		.name = "nokia-bluetooth",
+		.pm = &nokia_bluetooth_pm_ops,
+		.of_match_table = of_match_ptr(nokia_bluetooth_of_match),
+	},
+};
+
+module_serdev_device_driver(nokia_bluetooth_serdev_driver);
-- 
2.11.0

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

[Rob Herring]

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.

> + - reset-gpios: GPIO specifier, used to reset the BT module

Need to state active state.

> + - 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.

Also need to specify the active state.

> + - 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";
> +       };
> +};

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

[Rob Herring]

On Sat, Mar 4, 2017 at 5:58 AM, Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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.

> + - reset-gpios: GPIO specifier, used to reset the BT module

Need to state active state.

> + - 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.

Also need to specify the active state.

> + - 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";
> +       };
> +};
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 4109 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 4167 bytes --]

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-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Marcel Holtmann]

Hi Sebastian,

>>> 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”

I never liked the term “h4p”. It is fundamentally just “bluetooth”. What are our plans for Broadcom, Intel etc. Bluetooth UARTs?

Regards

Marcel

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

[Marcel Holtmann]

Hi Sebastian,

>>> 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-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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”

I never liked the term “h4p”. It is fundamentally just “bluetooth”. What are our plans for Broadcom, Intel etc. Bluetooth UARTs?

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1177 bytes --]

Hi Marcel,

On Tue, Mar 07, 2017 at 10:20:27PM +0100, Marcel Holtmann wrote:
>> [...]
>>
>> - compatible: should contain "nokia,h4p-bluetooth" as well as one of the following:
>> * "brcm,bcm2048-nokia"
>> * "ti,wl1271-blueooth-nokia”
> 
> I never liked the term “h4p”. It is fundamentally just
> “bluetooth”.

It's not me, that came up with the term, Nokia called their protocol
H4+ (for DT people not familiar with bluetooth - the standard
bluetooth UART protocol is named H4). I agree, that H4+ is
ambiguous, but with the "nokia," prefix it should be clear what
protocol is meant. I don't really mind, though. We can also just
list the individual chips and add all compatible strings to the
driver.

FWIW I remove the term from the driver leaving only a single
instance in the header ("AKA H4+").

> What are our plans for Broadcom, Intel etc. Bluetooth UARTs?

I think drivers, that need extra resources (like GPIOs) can switch to
serdev exclusively. The kernel needs ACPI/DT/boardcode data for those
devices anyways.

Iirc Rob is working on serdev patches for Broadcom and TI BT, so he may
provide more information.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1177 bytes --]

Hi Marcel,

On Tue, Mar 07, 2017 at 10:20:27PM +0100, Marcel Holtmann wrote:
>> [...]
>>
>> - compatible: should contain "nokia,h4p-bluetooth" as well as one of the following:
>> * "brcm,bcm2048-nokia"
>> * "ti,wl1271-blueooth-nokia”
> 
> I never liked the term “h4p”. It is fundamentally just
> “bluetooth”.

It's not me, that came up with the term, Nokia called their protocol
H4+ (for DT people not familiar with bluetooth - the standard
bluetooth UART protocol is named H4). I agree, that H4+ is
ambiguous, but with the "nokia," prefix it should be clear what
protocol is meant. I don't really mind, though. We can also just
list the individual chips and add all compatible strings to the
driver.

FWIW I remove the term from the driver leaving only a single
instance in the header ("AKA H4+").

> What are our plans for Broadcom, Intel etc. Bluetooth UARTs?

I think drivers, that need extra resources (like GPIOs) can switch to
serdev exclusively. The kernel needs ACPI/DT/boardcode data for those
devices anyways.

Iirc Rob is working on serdev patches for Broadcom and TI BT, so he may
provide more information.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Rob Herring]

On Tue, Mar 7, 2017 at 3:08 PM, Sebastian Reichel <sre@kernel.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@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"

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

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

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 2210 bytes --]

Hi!

> +struct hci_nokia_neg_hdr {
> +	__u8	dlen;
> +} __packed;
> +
> +struct hci_nokia_neg_cmd {
> +	__u8	ack;
> +	__u16	baud;
> +	__u16	unused1;
> +	__u8	proto;
> +	__u16	sys_clk;
> +	__u16	unused2;
> +} __packed;

__u8 -> u8? This is not exported to userspace...

> +#define BT_BAUDRATE_DIVIDER	384000000

Is this really divider?

> +	int init_error;
> +	struct completion init_completion;
> +
> +	uint8_t man_id;
> +	uint8_t ver_id;

u8...

> +static int nokia_wait_for_cts(struct hci_uart *hu, bool state,
> +				 int timeout_ms)
> +{
> +	struct nokia_bt_dev *btdev = hu->priv;
> +	struct device *dev = &btdev->serdev->dev;
> +	unsigned long timeout;
> +	bool signal;
> +
> +	timeout = jiffies + msecs_to_jiffies(timeout_ms);
> +	while (!time_after(jiffies, timeout)) {
> +		signal = serdev_device_get_cts(btdev->serdev);
> +		if (signal == state) {
> +			dev_dbg(dev, "wait for cts... received!");
> +			return 0;
> +		}
> +		usleep_range(1000, 2000);
> +	}
> +
> +	return -ETIMEDOUT;
> +}

Do we have devices where cts triggers interrupt?

> +	if (btdev->init_error < 0)
> +		return btdev->init_error;
> +
> +	/* Change to previously negotiated speed. Flow Control
> +	 * is disabled until bluetooth adapter is ready to avoid
> +	 * broken bytes being ready by the bluetooth adapter
> +	 */

Umm. I'd at dot at end of sentence... but still can't understand the
sentence.

"to avoid broken bytes being received."?

> +	evt = (struct hci_nokia_neg_evt *)skb_pull(skb, sizeof(*hdr));
> +
> +	if (evt->ack != NOKIA_NEG_ACK) {
> +		dev_err(dev, "Negotiation received: wrong reply");
> +		btdev->init_error = -EINVAL;
> +	}

But we still return success and trust the man_id / ver_id?

> +	pkt = (struct hci_nokia_alive_pkt *)skb_pull(skb, sizeof(*hdr));
> +
> +	if (pkt->mid != NOKIA_ALIVE_RESP) {
> +		dev_err(dev, "Alive received: invalid response: 0x%02x!",
> +			pkt->mid);
> +		btdev->init_error = -EINVAL;
> +		goto finish_alive;
> +	}

ret = -EINVAL?

Thanks!
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]