Linux Serial subsystem development
 help / color / mirror / Atom feed
* Re: [RFC 1/3] serdev: Add ACPI support
From: Frédéric Danis @ 2017-09-29 12:17 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Rob Herring, sre, loic.poulain, linux-bluetooth, linux-serial,
	linux-acpi, johan
In-Reply-To: <D48CDCC9-F039-4E80-BA12-5A579E13E09F@holtmann.org>

Hi Marcel,

Le 29/09/2017 à 14:00, Marcel Holtmann a écrit :
> Hi Fred,
>
>>>>> +#ifdef CONFIG_ACPI
>>>>> +static acpi_status acpi_serdev_register_device(struct serdev_controller *ctrl,
>>>>> +					    struct acpi_device *adev)
>>>>> +{
>>>>> +	struct serdev_device *serdev = NULL;
>>>>> +	int err;
>>>>> +
>>>>> +	if (acpi_bus_get_status(adev) || !adev->status.present ||
>>>>> +	    acpi_device_enumerated(adev))
>>>>> +		return AE_OK;
>>>>> +
>>>>> +	serdev = serdev_device_alloc(ctrl);
>>>>> +	if (!serdev) {
>>>>> +		dev_err(&ctrl->dev, "failed to allocate Serial device for %s\n",
>>>>> +			dev_name(&adev->dev));
>>>>> +		return AE_NO_MEMORY;
>>>>> +	}
>>>>> +
>>>>> +	ACPI_COMPANION_SET(&serdev->dev, adev);
>>>>> +	acpi_device_set_enumerated(adev);
>>>>> +
>>>>> +	err = serdev_device_add(serdev);
>>>>> +	if (err) {
>>>>> +		dev_err(&serdev->dev,
>>>>> +			"failure adding ACPI device. status %d\n", err);
>>>>> +		serdev_device_put(serdev);
>>>>> +	}
>>>>> +
>>>>> +	return AE_OK;
>>>>> +}
>>>>> +
>>>>> +static acpi_status acpi_serdev_add_device(acpi_handle handle, u32 level,
>>>>> +				       void *data, void **return_value)
>>>>> +{
>>>>> +	struct serdev_controller *ctrl = data;
>>>>> +	struct acpi_device *adev;
>>>>> +
>>>>> +	if (acpi_bus_get_device(handle, &adev))
>>>>> +		return AE_OK;
>>>>> +
>>>>> +	return acpi_serdev_register_device(ctrl, adev);
>>>>> +}
>>>>> +
>>>>> +static int acpi_serdev_register_devices(struct serdev_controller *ctrl)
>>>>> +{
>>>>> +	acpi_status status;
>>>>> +	acpi_handle handle;
>>>>> +
>>>>> +	handle = ACPI_HANDLE(ctrl->dev.parent);
>>>>> +	if (!handle)
>>>>> +		return -ENODEV;
>>>>> +
>>>>> +	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, 1,
>>>>> +				     acpi_serdev_add_device, NULL, ctrl, NULL);
>>>>> +	if (ACPI_FAILURE(status)) {
>>>>> +		dev_warn(&ctrl->dev, "failed to enumerate Serial slaves\n");
>>>>> +		return -ENODEV;
>>>>> +	}
>>>>> +
>>>>> +	return 0;
>>>>> +}
>>>> how are we ensuring that we only take UART devices into account here?
>>> acpi_serdev_add_device() callback will only take into account entries without enumerated flag set.
>>> This flags is set for all entries during ACPI scan, except for SPI and I2C serial devices, and for UART with 2nd patch in the series.
>> sounds good to me. Can you respin this series with the other comments addressed and maybe add some extra comments in the code or the commit message to make this clear.
> any updates on this?

While working on it, I tried to add the PM support 
(bcm_serdev_driver.driver.pm = &bcm_pm_ops) but it ends up in kernel 
freeze during suspend to ram test.
I may have found the problem but do not have access to T100 now to test 
it (this may take some times as I work on it on spare time).

Regards,

Fred

^ permalink raw reply

* [PATCH 0/2] HSCIF tweaks support
From: Ulrich Hecht @ 2017-09-29 13:08 UTC (permalink / raw)
  To: linux-renesas-soc, geert; +Cc: wsa, linux-serial, magnus.damm, Ulrich Hecht

Hi!

These patches enable tweaking of the HSCIF RX FIFO timeout and sampling
point via sysfs. I have put them in one series because they have textual
dependencies and implement similar functionality.

"serial: sh-sci: Support for variable HSCIF hardware RX timeout" includes
minor touch-ups suggested in Geert's review of the standalone version.

CU
Uli


Ulrich Hecht (2):
  serial: sh-sci: Support for variable HSCIF hardware RX timeout
  serial: sh-sci: Support for HSCIF RX sampling point adjustment

 drivers/tty/serial/sh-sci.c | 114 +++++++++++++++++++++++++++++++++++++-------
 drivers/tty/serial/sh-sci.h |   7 +++
 2 files changed, 104 insertions(+), 17 deletions(-)

-- 
2.7.4

^ permalink raw reply

* [PATCH 1/2] serial: sh-sci: Support for variable HSCIF hardware RX timeout
From: Ulrich Hecht @ 2017-09-29 13:08 UTC (permalink / raw)
  To: linux-renesas-soc, geert; +Cc: wsa, linux-serial, magnus.damm, Ulrich Hecht
In-Reply-To: <1506690534-27302-1-git-send-email-ulrich.hecht+renesas@gmail.com>

HSCIF has facilities that allow changing the timeout after which an RX
interrupt is triggered even if the FIFO is not filled. This patch allows
changing the default (15 bits of silence) using the existing sysfs
attribute "rx_fifo_timeout".

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
 drivers/tty/serial/sh-sci.c | 52 ++++++++++++++++++++++++++++++++-------------
 drivers/tty/serial/sh-sci.h |  3 +++
 2 files changed, 40 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 784dd42..41bf910 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -152,6 +152,7 @@ struct sci_port {
 	int				rx_trigger;
 	struct timer_list		rx_fifo_timer;
 	int				rx_fifo_timeout;
+	u16				hscif_tot;
 
 	bool has_rtscts;
 	bool autorts;
@@ -1107,8 +1108,14 @@ static ssize_t rx_fifo_timeout_show(struct device *dev,
 {
 	struct uart_port *port = dev_get_drvdata(dev);
 	struct sci_port *sci = to_sci_port(port);
+	int v;
 
-	return sprintf(buf, "%d\n", sci->rx_fifo_timeout);
+	if (port->type == PORT_HSCIF)
+		v = sci->hscif_tot >> HSSCR_TOT_SHIFT;
+	else
+		v = sci->rx_fifo_timeout;
+
+	return sprintf(buf, "%d\n", v);
 }
 
 static ssize_t rx_fifo_timeout_store(struct device *dev,
@@ -1124,11 +1131,19 @@ static ssize_t rx_fifo_timeout_store(struct device *dev,
 	ret = kstrtol(buf, 0, &r);
 	if (ret)
 		return ret;
-	sci->rx_fifo_timeout = r;
-	scif_set_rtrg(port, 1);
-	if (r > 0)
-		setup_timer(&sci->rx_fifo_timer, rx_fifo_timer_fn,
-			    (unsigned long)sci);
+
+	if (port->type == PORT_HSCIF) {
+		if (r < 0 || r > 3)
+			return -EINVAL;
+		sci->hscif_tot = r << HSSCR_TOT_SHIFT;
+	} else {
+		sci->rx_fifo_timeout = r;
+		scif_set_rtrg(port, 1);
+		if (r > 0)
+			setup_timer(&sci->rx_fifo_timer, rx_fifo_timer_fn,
+				    (unsigned long)sci);
+	}
+
 	return count;
 }
 
@@ -2037,9 +2052,13 @@ static void sci_shutdown(struct uart_port *port)
 	spin_lock_irqsave(&port->lock, flags);
 	sci_stop_rx(port);
 	sci_stop_tx(port);
-	/* Stop RX and TX, disable related interrupts, keep clock source */
+	/*
+	 * Stop RX and TX, disable related interrupts, keep clock source
+	 * and HSCIF TOT bits
+	 */
 	scr = serial_port_in(port, SCSCR);
-	serial_port_out(port, SCSCR, scr & (SCSCR_CKE1 | SCSCR_CKE0));
+	serial_port_out(port, SCSCR, scr &
+			(SCSCR_CKE1 | SCSCR_CKE0 | s->hscif_tot));
 	spin_unlock_irqrestore(&port->lock, flags);
 
 #ifdef CONFIG_SERIAL_SH_SCI_DMA
@@ -2186,7 +2205,7 @@ static void sci_reset(struct uart_port *port)
 	unsigned int status;
 	struct sci_port *s = to_sci_port(port);
 
-	serial_port_out(port, SCSCR, 0x00);	/* TE=0, RE=0, CKE1=0 */
+	serial_port_out(port, SCSCR, s->hscif_tot);	/* TE=0, RE=0, CKE1=0 */
 
 	reg = sci_getreg(port, SCFCR);
 	if (reg->size)
@@ -2356,7 +2375,7 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
 		dev_dbg(port->dev,
 			 "SCR 0x%x SMR 0x%x BRR %u CKS 0x%x DL %u SRR %u\n",
 			 scr_val, smr_val, brr, sccks, dl, srr);
-		serial_port_out(port, SCSCR, scr_val);
+		serial_port_out(port, SCSCR, scr_val | s->hscif_tot);
 		serial_port_out(port, SCSMR, smr_val);
 		serial_port_out(port, SCBRR, brr);
 		if (sci_getreg(port, HSSRR)->size)
@@ -2370,7 +2389,7 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
 		smr_val |= serial_port_in(port, SCSMR) &
 			   (SCSMR_CKEDG | SCSMR_SRC_MASK | SCSMR_CKS);
 		dev_dbg(port->dev, "SCR 0x%x SMR 0x%x\n", scr_val, smr_val);
-		serial_port_out(port, SCSCR, scr_val);
+		serial_port_out(port, SCSCR, scr_val | s->hscif_tot);
 		serial_port_out(port, SCSMR, smr_val);
 	}
 
@@ -2407,7 +2426,7 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
 	scr_val |= SCSCR_RE | SCSCR_TE |
 		   (s->cfg->scscr & ~(SCSCR_CKE1 | SCSCR_CKE0));
 	dev_dbg(port->dev, "SCSCR 0x%x\n", scr_val);
-	serial_port_out(port, SCSCR, scr_val);
+	serial_port_out(port, SCSCR, scr_val | s->hscif_tot);
 	if ((srr + 1 == 5) &&
 	    (port->type == PORT_SCIFA || port->type == PORT_SCIFB)) {
 		/*
@@ -2773,6 +2792,7 @@ static int sci_init_single(struct platform_device *dev,
 	}
 
 	sci_port->rx_fifo_timeout = 0;
+	sci_port->hscif_tot = 0;
 
 	/* SCIFA on sh7723 and sh7724 need a custom sampling rate that doesn't
 	 * match the SoC datasheet, this should be investigated. Let platform
@@ -2860,7 +2880,7 @@ static void serial_console_write(struct console *co, const char *s,
 	ctrl_temp = SCSCR_RE | SCSCR_TE |
 		    (sci_port->cfg->scscr & ~(SCSCR_CKE1 | SCSCR_CKE0)) |
 		    (ctrl & (SCSCR_CKE1 | SCSCR_CKE0));
-	serial_port_out(port, SCSCR, ctrl_temp);
+	serial_port_out(port, SCSCR, ctrl_temp | sci_port->hscif_tot);
 
 	uart_console_write(port, s, count, serial_console_putchar);
 
@@ -2988,7 +3008,8 @@ static int sci_remove(struct platform_device *dev)
 		sysfs_remove_file(&dev->dev.kobj,
 				  &dev_attr_rx_fifo_trigger.attr);
 	}
-	if (port->port.type == PORT_SCIFA || port->port.type == PORT_SCIFB) {
+	if (port->port.type == PORT_SCIFA || port->port.type == PORT_SCIFB ||
+	    port->port.type == PORT_HSCIF) {
 		sysfs_remove_file(&dev->dev.kobj,
 				  &dev_attr_rx_fifo_timeout.attr);
 	}
@@ -3173,7 +3194,8 @@ static int sci_probe(struct platform_device *dev)
 		if (ret)
 			return ret;
 	}
-	if (sp->port.type == PORT_SCIFA || sp->port.type ==  PORT_SCIFB) {
+	if (sp->port.type == PORT_SCIFA || sp->port.type == PORT_SCIFB ||
+	    sp->port.type == PORT_HSCIF) {
 		ret = sysfs_create_file(&dev->dev.kobj,
 				&dev_attr_rx_fifo_timeout.attr);
 		if (ret) {
diff --git a/drivers/tty/serial/sh-sci.h b/drivers/tty/serial/sh-sci.h
index 971b2ab..2b708cc 100644
--- a/drivers/tty/serial/sh-sci.h
+++ b/drivers/tty/serial/sh-sci.h
@@ -62,6 +62,9 @@ enum {
 #define SCSCR_TDRQE	BIT(15)	/* Tx Data Transfer Request Enable */
 #define SCSCR_RDRQE	BIT(14)	/* Rx Data Transfer Request Enable */
 
+/* Serial Control Register, HSCIF-only bits */
+#define HSSCR_TOT_SHIFT	14
+
 /* SCxSR (Serial Status Register) on SCI */
 #define SCI_TDRE	BIT(7)	/* Transmit Data Register Empty */
 #define SCI_RDRF	BIT(6)	/* Receive Data Register Full */
-- 
2.7.4

^ permalink raw reply related

* [PATCH 2/2] serial: sh-sci: Support for HSCIF RX sampling point adjustment
From: Ulrich Hecht @ 2017-09-29 13:08 UTC (permalink / raw)
  To: linux-renesas-soc, geert; +Cc: wsa, linux-serial, magnus.damm, Ulrich Hecht
In-Reply-To: <1506690534-27302-1-git-send-email-ulrich.hecht+renesas@gmail.com>

HSCIF has facilities that allow moving the RX sampling point by between
-8 and 7 sampling cycles (one sampling cycles equals 1/15 of a bit
by default) to improve the error margin in case of slightly mismatched
bit rates between sender and receiver.

This patch allows changing the default (0, meaning center) using the
sysfs attribute "rx_sampling_point".

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
 drivers/tty/serial/sh-sci.c | 62 +++++++++++++++++++++++++++++++++++++++++++--
 drivers/tty/serial/sh-sci.h |  4 +++
 2 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 41bf910..924a393 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -153,6 +153,7 @@ struct sci_port {
 	struct timer_list		rx_fifo_timer;
 	int				rx_fifo_timeout;
 	u16				hscif_tot;
+	u16				hscif_srhp;
 
 	bool has_rtscts;
 	bool autorts;
@@ -992,6 +993,42 @@ static int sci_handle_breaks(struct uart_port *port)
 	return copied;
 }
 
+static ssize_t rx_sampling_point_show(struct device *dev,
+			       struct device_attribute *attr,
+			       char *buf)
+{
+	struct uart_port *port = dev_get_drvdata(dev);
+	struct sci_port *sci = to_sci_port(port);
+
+	return sprintf(buf, "%d\n", sci->hscif_srhp);
+}
+
+static ssize_t rx_sampling_point_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf,
+				size_t count)
+{
+	struct uart_port *port = dev_get_drvdata(dev);
+	struct sci_port *sci = to_sci_port(port);
+	int ret;
+	long r;
+
+	ret = kstrtol(buf, 0, &r);
+	if (ret)
+		return ret;
+
+	if (r < -8 || r > 7)
+		return -EINVAL;
+
+	sci->hscif_srhp = r;
+
+	return count;
+}
+
+static DEVICE_ATTR(rx_sampling_point, 0644,
+		   rx_sampling_point_show,
+		   rx_sampling_point_store);
+
 static int scif_set_rtrg(struct uart_port *port, int rx_trig)
 {
 	unsigned int bits;
@@ -2378,8 +2415,6 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
 		serial_port_out(port, SCSCR, scr_val | s->hscif_tot);
 		serial_port_out(port, SCSMR, smr_val);
 		serial_port_out(port, SCBRR, brr);
-		if (sci_getreg(port, HSSRR)->size)
-			serial_port_out(port, HSSRR, srr | HSCIF_SRE);
 
 		/* Wait one bit interval */
 		udelay((1000000 + (baud - 1)) / baud);
@@ -2393,6 +2428,18 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
 		serial_port_out(port, SCSMR, smr_val);
 	}
 
+	if (sci_getreg(port, HSSRR)->size) {
+		u16 hssrr = srr | HSCIF_SRE;
+
+		if (s->hscif_srhp) {
+			u16 srhp = (s->hscif_srhp << HSCIF_SRHP_SHIFT) &
+				   HSCIF_SRHP_MASK;
+
+			hssrr |= HSCIF_SRDE | srhp;
+		}
+		serial_port_out(port, HSSRR, hssrr);
+	}
+
 	sci_init_pins(port, termios->c_cflag);
 
 	port->status &= ~UPSTAT_AUTOCTS;
@@ -2793,6 +2840,7 @@ static int sci_init_single(struct platform_device *dev,
 
 	sci_port->rx_fifo_timeout = 0;
 	sci_port->hscif_tot = 0;
+	sci_port->hscif_shrp = 0;
 
 	/* SCIFA on sh7723 and sh7724 need a custom sampling rate that doesn't
 	 * match the SoC datasheet, this should be investigated. Let platform
@@ -3013,6 +3061,10 @@ static int sci_remove(struct platform_device *dev)
 		sysfs_remove_file(&dev->dev.kobj,
 				  &dev_attr_rx_fifo_timeout.attr);
 	}
+	if (port->port.type == PORT_HSCIF) {
+		sysfs_remove_file(&dev->dev.kobj,
+				  &dev_attr_rx_sampling_point.attr);
+	}
 
 	return 0;
 }
@@ -3206,6 +3258,12 @@ static int sci_probe(struct platform_device *dev)
 			return ret;
 		}
 	}
+	if (sp->port.type == PORT_HSCIF) {
+		ret = sysfs_create_file(&dev->dev.kobj,
+				&dev_attr_rx_sampling_point.attr);
+		if (ret)
+			return ret;
+	}
 
 #ifdef CONFIG_SH_STANDARD_BIOS
 	sh_bios_gdb_detach();
diff --git a/drivers/tty/serial/sh-sci.h b/drivers/tty/serial/sh-sci.h
index 2b708cc..80958b2 100644
--- a/drivers/tty/serial/sh-sci.h
+++ b/drivers/tty/serial/sh-sci.h
@@ -129,6 +129,10 @@ enum {
 
 /* HSSRR HSCIF */
 #define HSCIF_SRE	BIT(15)	/* Sampling Rate Register Enable */
+#define HSCIF_SRDE	BIT(14)	/* Sampling Point Register Enable */
+
+#define HSCIF_SRHP_SHIFT	8
+#define HSCIF_SRHP_MASK		0x0f00
 
 /* SCPCR (Serial Port Control Register), SCIFA/SCIFB only */
 #define SCPCR_RTSC	BIT(4)	/* Serial Port RTS# Pin / Output Pin */
-- 
2.7.4

^ permalink raw reply related

* Re: [RFC,3/3] Bluetooth: hci_bcm: Add ACPI serdev support for BCM2E39
From: Hans de Goede @ 2017-10-02  7:07 UTC (permalink / raw)
  To: Frédéric Danis, robh-DgEjT+Ai2ygdnm+yROfE0A,
	marcel-kz+m5ild9QBg9hUCZPvPmw, sre-DgEjT+Ai2ygdnm+yROfE0A,
	loic.poulain-Re5JQEeQqe8AvxtiuMwx3w
  Cc: linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1504786214-1866-4-git-send-email-frederic.danis.oss-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

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

Hi,

First of all Frédéric, thank you very much for working on this,
this is a very much needed patch series.

I've been testing this on a GPD pocket which has a BCM2E7E
ACPI device for the BT which is part of the BCM43562A wifi/bt
combo on this device.

2 remarks:

1) The following errors show up with this series:

[   87.059091] synth uevent: /devices/pci0000:00/8086228A:00/serial1: failed to send uevent
[   87.059097] serial serial1: uevent: failed to send synthetic uevent
[   87.059178] synth uevent: /devices/pci0000:00/8086228A:01/serial2: failed to send uevent
[   87.059180] serial serial2: uevent: failed to send synthetic uevent
[   87.062665] synth uevent: /devices/pnp0/00:01/serial0: failed to send uevent
[   87.062671] serial serial0: uevent: failed to send synthetic uevent

This needs to be fixed :)  I haven't looked into this yet.

2) As I already suspected when reading the patches, we cannot
move the ACPI ids over 1 by 1. The first 2 patches in the series
cause all BCM2E* ACPI devices to no longer get enumerated as
platform devices, instead they now get enumerated as serdevs.

So adding only the known to work ACPI ids to the acpi_match_table
for the bcm_serdev_driver has the undesirable result that it makes
sure that the other ACPI-ids will not work, as there are not
platform devices instantiated for the platform driver to bind to.

I started with simplifying your patch to this:

 From f8728f5440c85f7e5584ac1eafa1b090b2042276 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Danis?= <frederic.danis.oss-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Thu, 7 Sep 2017 14:10:14 +0200
Subject: [PATCH] Bluetooth: hci_bcm: Make ACPI enumerated HCIs use serdev
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Now that the ACPI subsys instantiates ACPI enumerated HCIs as serdevs,
the ACPI ids for them should be part of the bcm_serdev_driver's
acpi_match_table.

Signed-off-by: Frédéric Danis <frederic.danis.oss-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
[hdegoede: Move all ACPI ids over]
Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
   drivers/bluetooth/hci_bcm.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 2285ca673ae3..4db777bb0049 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -942,7 +942,6 @@ static struct platform_driver bcm_driver = {
   	.remove = bcm_remove,
   	.driver = {
   		.name = "hci_bcm",
-		.acpi_match_table = ACPI_PTR(bcm_acpi_match),
   		.pm = &bcm_pm_ops,
   	},
   };
@@ -988,6 +987,7 @@ static struct serdev_device_driver bcm_serdev_driver = {
   	.driver = {
   		.name = "hci_uart_bcm",
   		.of_match_table = of_match_ptr(bcm_bluetooth_of_match),
+		.acpi_match_table = ACPI_PTR(bcm_acpi_match),
   	},
   };

-- 
2.14.2

Unfortunately that is not good enough. The platform driver code also
has (runtime) pm support using gpios and a host-wake interrupt, which
the serdev code all lacks and without driving the shutdown gpio to not
shutdown the BT device connected to the UART we not only are lacking
pm support, but BT does not work at all.

So yesterday evening I did a patch to merge all that into the serdev hci_bcm
code and throw away the platform code as I don't think any users will be left
after this. With this functionality for the BCM2E7E ACPI device is restored
again, without needing to do a btattach from userspace, which is great :)

I've attached the resulting patch. In the mean time I've realized that
this approach is going to make merging hard. So I'm going to redo the changes
so that we can have both a complete (with runtime-pm, gpios, etc.) serdev
driver and keep the platform driver for now. Then we can first merge the
hci_bcm changes to make the serdev driver side complete and once those are
merged merge the matching ACPI subsys changes without having a window in
between where things will not work.

###

On a related note this series does expose a module load ordering issue.
With this series and with the dw_dmac driver built as module I get:

[   80.239270] ttyS4 - failed to request DMA
[   80.316338] dw_dmac INTL9C60:00: DesignWare DMA Controller, 8 channels
[   80.353639] dw_dmac INTL9C60:01: DesignWare DMA Controller, 8 channels

Building in dw_dmac is a workaround for this, anyone has any idea how to
fix this ?

Regards,

Hans








On 07-09-17 14:10, Frédéric Danis wrote:
> Signed-off-by: Frédéric Danis <frederic.danis.oss-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>   drivers/bluetooth/hci_bcm.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
> index 2e358cc..b1cf07e 100644
> --- a/drivers/bluetooth/hci_bcm.c
> +++ b/drivers/bluetooth/hci_bcm.c
> @@ -922,7 +922,6 @@ static const struct hci_uart_proto bcm_proto = {
>   #ifdef CONFIG_ACPI
>   static const struct acpi_device_id bcm_acpi_match[] = {
>   	{ "BCM2E1A", (kernel_ulong_t)&acpi_bcm_int_last_gpios },
> -	{ "BCM2E39", (kernel_ulong_t)&acpi_bcm_int_last_gpios },
>   	{ "BCM2E3A", (kernel_ulong_t)&acpi_bcm_int_last_gpios },
>   	{ "BCM2E3D", (kernel_ulong_t)&acpi_bcm_int_last_gpios },
>   	{ "BCM2E3F", (kernel_ulong_t)&acpi_bcm_int_last_gpios },
> @@ -942,6 +941,14 @@ static const struct acpi_device_id bcm_acpi_match[] = {
>   MODULE_DEVICE_TABLE(acpi, bcm_acpi_match);
>   #endif
>   
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id bcm_serdev_acpi_match[] = {
> +	{ "BCM2E39", (kernel_ulong_t)&acpi_bcm_int_last_gpios },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(acpi, bcm_serdev_acpi_match);
> +#endif
> +
>   /* Platform suspend and resume callbacks */
>   static const struct dev_pm_ops bcm_pm_ops = {
>   	SET_SYSTEM_SLEEP_PM_OPS(bcm_suspend, bcm_resume)
> @@ -999,6 +1006,7 @@ static struct serdev_device_driver bcm_serdev_driver = {
>   	.driver = {
>   		.name = "hci_uart_bcm",
>   		.of_match_table = of_match_ptr(bcm_bluetooth_of_match),
> +		.acpi_match_table = ACPI_PTR(bcm_serdev_acpi_match),
>   	},
>   };
>   
> 


[-- Attachment #2: 0001-Bluetooth-hci_bcm-Change-from-platform-driver-into-s.patch --]
[-- Type: text/x-patch, Size: 20831 bytes --]

>From ce62881000fc076326220356036b65074c385a5e Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Date: Sun, 1 Oct 2017 23:17:34 +0200
Subject: [PATCH] Bluetooth: hci_bcm: Change from platform driver into serdev
 driver

Now that the ACPI subsys instantiates ACPI enumerated HCIs as serdevs,
we no longer need a separate platform-driver and serdev-driver, but
we do need the platform-driver's gpio, irq and pm functionality in the
serdev case now.

Merge the platform and serdev driver code into a single serdev-driver.

Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 drivers/bluetooth/hci_bcm.c | 452 ++++++++++++++------------------------------
 1 files changed, 138 insertions(+), 316 deletions(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 2285ca673ae3..bf5f2629257a 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -29,7 +29,6 @@
 #include <linux/acpi.h>
 #include <linux/of.h>
 #include <linux/property.h>
-#include <linux/platform_device.h>
 #include <linux/clk.h>
 #include <linux/gpio/consumer.h>
 #include <linux/tty.h>
@@ -54,9 +53,7 @@
 
 /* platform device driver resources */
 struct bcm_device {
-	struct list_head	list;
-
-	struct platform_device	*pdev;
+	struct hci_uart hu;
 
 	const char		*name;
 	struct gpio_desc	*device_wakeup;
@@ -69,16 +66,6 @@ struct bcm_device {
 	u32			oper_speed;
 	int			irq;
 	bool			irq_active_low;
-
-#ifdef CONFIG_PM
-	struct hci_uart		*hu;
-	bool			is_suspended; /* suspend/resume flag */
-#endif
-};
-
-/* serdev driver resources */
-struct bcm_serdev {
-	struct hci_uart hu;
 };
 
 /* generic bcm uart resources */
@@ -89,18 +76,6 @@ struct bcm_data {
 	struct bcm_device	*dev;
 };
 
-/* List of BCM BT UART devices */
-static DEFINE_MUTEX(bcm_device_lock);
-static LIST_HEAD(bcm_device_list);
-
-static inline void host_set_baudrate(struct hci_uart *hu, unsigned int speed)
-{
-	if (hu->serdev)
-		serdev_device_set_baudrate(hu->serdev, speed);
-	else
-		hci_uart_set_baudrate(hu, speed);
-}
-
 static int bcm_set_baudrate(struct hci_uart *hu, unsigned int speed)
 {
 	struct hci_dev *hdev = hu->hdev;
@@ -150,21 +125,6 @@ static int bcm_set_baudrate(struct hci_uart *hu, unsigned int speed)
 	return 0;
 }
 
-/* bcm_device_exists should be protected by bcm_device_lock */
-static bool bcm_device_exists(struct bcm_device *device)
-{
-	struct list_head *p;
-
-	list_for_each(p, &bcm_device_list) {
-		struct bcm_device *dev = list_entry(p, struct bcm_device, list);
-
-		if (device == dev)
-			return true;
-	}
-
-	return false;
-}
-
 static int bcm_gpio_set_power(struct bcm_device *dev, bool powered)
 {
 	if (powered && !IS_ERR(dev->clk) && !dev->clk_enabled)
@@ -185,12 +145,13 @@ static int bcm_gpio_set_power(struct bcm_device *dev, bool powered)
 static irqreturn_t bcm_host_wake(int irq, void *data)
 {
 	struct bcm_device *bdev = data;
+	struct device *dev = &bdev->hu.serdev->dev;
 
 	bt_dev_dbg(bdev, "Host wake IRQ");
 
-	pm_runtime_get(&bdev->pdev->dev);
-	pm_runtime_mark_last_busy(&bdev->pdev->dev);
-	pm_runtime_put_autosuspend(&bdev->pdev->dev);
+	pm_runtime_get(dev);
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
 
 	return IRQ_HANDLED;
 }
@@ -198,39 +159,26 @@ static irqreturn_t bcm_host_wake(int irq, void *data)
 static int bcm_request_irq(struct bcm_data *bcm)
 {
 	struct bcm_device *bdev = bcm->dev;
+	struct device *dev = &bdev->hu.serdev->dev;
 	int err;
 
-	/* If this is not a platform device, do not enable PM functionalities */
-	mutex_lock(&bcm_device_lock);
-	if (!bcm_device_exists(bdev)) {
-		err = -ENODEV;
-		goto unlock;
-	}
+	if (bdev->irq <= 0)
+		return -EOPNOTSUPP;
 
-	if (bdev->irq <= 0) {
-		err = -EOPNOTSUPP;
-		goto unlock;
-	}
-
-	err = devm_request_irq(&bdev->pdev->dev, bdev->irq, bcm_host_wake,
+	err = devm_request_irq(dev, bdev->irq, bcm_host_wake,
 			       bdev->irq_active_low ? IRQF_TRIGGER_FALLING :
 						      IRQF_TRIGGER_RISING,
 			       "host_wake", bdev);
 	if (err)
-		goto unlock;
-
-	device_init_wakeup(&bdev->pdev->dev, true);
-
-	pm_runtime_set_autosuspend_delay(&bdev->pdev->dev,
-					 BCM_AUTOSUSPEND_DELAY);
-	pm_runtime_use_autosuspend(&bdev->pdev->dev);
-	pm_runtime_set_active(&bdev->pdev->dev);
-	pm_runtime_enable(&bdev->pdev->dev);
+		return err;
 
-unlock:
-	mutex_unlock(&bcm_device_lock);
+	device_init_wakeup(dev, true);
+	pm_runtime_set_autosuspend_delay(dev, BCM_AUTOSUSPEND_DELAY);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
 
-	return err;
+	return 0;
 }
 
 static const struct bcm_set_sleep_mode default_sleep_params = {
@@ -300,8 +248,8 @@ static int bcm_set_diag(struct hci_dev *hdev, bool enable)
 
 static int bcm_open(struct hci_uart *hu)
 {
+	struct bcm_device *bdev;
 	struct bcm_data *bcm;
-	struct list_head *p;
 
 	bt_dev_dbg(hu->hdev, "hu %p", hu);
 
@@ -311,41 +259,15 @@ static int bcm_open(struct hci_uart *hu)
 
 	skb_queue_head_init(&bcm->txq);
 
+	bdev = serdev_device_get_drvdata(hu->serdev);
+	bcm->dev = bdev;
+	hu->init_speed = bdev->init_speed;
+	hu->oper_speed = bdev->oper_speed;
 	hu->priv = bcm;
 
-	/* If this is a serdev defined device, then only use
-	 * serdev open primitive and skip the rest.
-	 */
-	if (hu->serdev) {
-		serdev_device_open(hu->serdev);
-		goto out;
-	}
-
-	if (!hu->tty->dev)
-		goto out;
-
-	mutex_lock(&bcm_device_lock);
-	list_for_each(p, &bcm_device_list) {
-		struct bcm_device *dev = list_entry(p, struct bcm_device, list);
-
-		/* Retrieve saved bcm_device based on parent of the
-		 * platform device (saved during device probe) and
-		 * parent of tty device used by hci_uart
-		 */
-		if (hu->tty->dev->parent == dev->pdev->dev.parent) {
-			bcm->dev = dev;
-			hu->init_speed = dev->init_speed;
-			hu->oper_speed = dev->oper_speed;
-#ifdef CONFIG_PM
-			dev->hu = hu;
-#endif
-			bcm_gpio_set_power(bcm->dev, true);
-			break;
-		}
-	}
+	serdev_device_open(hu->serdev);
+	bcm_gpio_set_power(bdev, true);
 
-	mutex_unlock(&bcm_device_lock);
-out:
 	return 0;
 }
 
@@ -353,32 +275,22 @@ static int bcm_close(struct hci_uart *hu)
 {
 	struct bcm_data *bcm = hu->priv;
 	struct bcm_device *bdev = bcm->dev;
+	struct device *dev = &bdev->hu.serdev->dev;
 
 	bt_dev_dbg(hu->hdev, "hu %p", hu);
 
-	/* If this is a serdev defined device, only use serdev
-	 * close primitive and then continue as usual.
-	 */
-	if (hu->serdev)
-		serdev_device_close(hu->serdev);
+	serdev_device_close(hu->serdev);
 
-	/* Protect bcm->dev against removal of the device or driver */
-	mutex_lock(&bcm_device_lock);
-	if (bcm_device_exists(bdev)) {
-		bcm_gpio_set_power(bdev, false);
+	bcm_gpio_set_power(bdev, false);
 #ifdef CONFIG_PM
-		pm_runtime_disable(&bdev->pdev->dev);
-		pm_runtime_set_suspended(&bdev->pdev->dev);
-
-		if (device_can_wakeup(&bdev->pdev->dev)) {
-			devm_free_irq(&bdev->pdev->dev, bdev->irq, bdev);
-			device_init_wakeup(&bdev->pdev->dev, false);
-		}
+	pm_runtime_disable(dev);
+	pm_runtime_set_suspended(dev);
 
-		bdev->hu = NULL;
-#endif
+	if (device_can_wakeup(dev)) {
+		devm_free_irq(dev, bdev->irq, bdev);
+		device_init_wakeup(dev, false);
 	}
-	mutex_unlock(&bcm_device_lock);
+#endif
 
 	skb_queue_purge(&bcm->txq);
 	kfree_skb(bcm->rx_skb);
@@ -437,7 +349,7 @@ static int bcm_setup(struct hci_uart *hu)
 		speed = 0;
 
 	if (speed)
-		host_set_baudrate(hu, speed);
+		serdev_device_set_baudrate(hu->serdev, speed);
 
 	/* Operational speed if any */
 	if (hu->oper_speed)
@@ -450,7 +362,7 @@ static int bcm_setup(struct hci_uart *hu)
 	if (speed) {
 		err = bcm_set_baudrate(hu, speed);
 		if (!err)
-			host_set_baudrate(hu, speed);
+			serdev_device_set_baudrate(hu->serdev, speed);
 	}
 
 finalize:
@@ -491,6 +403,7 @@ static const struct h4_recv_pkt bcm_recv_pkts[] = {
 static int bcm_recv(struct hci_uart *hu, const void *data, int count)
 {
 	struct bcm_data *bcm = hu->priv;
+	struct device *dev = &bcm->dev->hu.serdev->dev;
 
 	if (!test_bit(HCI_UART_REGISTERED, &hu->flags))
 		return -EUNATCH;
@@ -504,13 +417,9 @@ static int bcm_recv(struct hci_uart *hu, const void *data, int count)
 		return err;
 	} else if (!bcm->rx_skb) {
 		/* Delay auto-suspend when receiving completed packet */
-		mutex_lock(&bcm_device_lock);
-		if (bcm->dev && bcm_device_exists(bcm->dev)) {
-			pm_runtime_get(&bcm->dev->pdev->dev);
-			pm_runtime_mark_last_busy(&bcm->dev->pdev->dev);
-			pm_runtime_put_autosuspend(&bcm->dev->pdev->dev);
-		}
-		mutex_unlock(&bcm_device_lock);
+		pm_runtime_get(dev);
+		pm_runtime_mark_last_busy(dev);
+		pm_runtime_put_autosuspend(dev);
 	}
 
 	return count;
@@ -532,25 +441,15 @@ static int bcm_enqueue(struct hci_uart *hu, struct sk_buff *skb)
 static struct sk_buff *bcm_dequeue(struct hci_uart *hu)
 {
 	struct bcm_data *bcm = hu->priv;
+	struct device *dev = &bcm->dev->hu.serdev->dev;
 	struct sk_buff *skb = NULL;
-	struct bcm_device *bdev = NULL;
-
-	mutex_lock(&bcm_device_lock);
 
-	if (bcm_device_exists(bcm->dev)) {
-		bdev = bcm->dev;
-		pm_runtime_get_sync(&bdev->pdev->dev);
-		/* Shall be resumed here */
-	}
+	pm_runtime_get_sync(dev);
 
 	skb = skb_dequeue(&bcm->txq);
 
-	if (bdev) {
-		pm_runtime_mark_last_busy(&bdev->pdev->dev);
-		pm_runtime_put_autosuspend(&bdev->pdev->dev);
-	}
-
-	mutex_unlock(&bcm_device_lock);
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
 
 	return skb;
 }
@@ -558,16 +457,12 @@ static struct sk_buff *bcm_dequeue(struct hci_uart *hu)
 #ifdef CONFIG_PM
 static int bcm_suspend_device(struct device *dev)
 {
-	struct bcm_device *bdev = platform_get_drvdata(to_platform_device(dev));
+	struct bcm_device *bdev =
+		serdev_device_get_drvdata(to_serdev_device(dev));
 
 	bt_dev_dbg(bdev, "");
 
-	if (!bdev->is_suspended && bdev->hu) {
-		hci_uart_set_flow_control(bdev->hu, true);
-
-		/* Once this returns, driver suspends BT via GPIO */
-		bdev->is_suspended = true;
-	}
+	serdev_device_set_flow_control(bdev->hu.serdev, false);
 
 	/* Suspend the device */
 	if (bdev->device_wakeup) {
@@ -581,7 +476,8 @@ static int bcm_suspend_device(struct device *dev)
 
 static int bcm_resume_device(struct device *dev)
 {
-	struct bcm_device *bdev = platform_get_drvdata(to_platform_device(dev));
+	struct bcm_device *bdev =
+		serdev_device_get_drvdata(to_serdev_device(dev));
 
 	bt_dev_dbg(bdev, "");
 
@@ -592,11 +488,7 @@ static int bcm_resume_device(struct device *dev)
 	}
 
 	/* When this executes, the device has woken up already */
-	if (bdev->is_suspended && bdev->hu) {
-		bdev->is_suspended = false;
-
-		hci_uart_set_flow_control(bdev->hu, false);
-	}
+	serdev_device_set_flow_control(bdev->hu.serdev, true);
 
 	return 0;
 }
@@ -606,61 +498,39 @@ static int bcm_resume_device(struct device *dev)
 /* Platform suspend callback */
 static int bcm_suspend(struct device *dev)
 {
-	struct bcm_device *bdev = platform_get_drvdata(to_platform_device(dev));
+	struct bcm_device *bdev =
+		serdev_device_get_drvdata(to_serdev_device(dev));
 	int error;
 
-	bt_dev_dbg(bdev, "suspend: is_suspended %d", bdev->is_suspended);
-
-	/* bcm_suspend can be called at any time as long as platform device is
-	 * bound, so it should use bcm_device_lock to protect access to hci_uart
-	 * and device_wake-up GPIO.
-	 */
-	mutex_lock(&bcm_device_lock);
-
-	if (!bdev->hu)
-		goto unlock;
+	bt_dev_dbg(bdev, "suspend");
 
 	if (pm_runtime_active(dev))
 		bcm_suspend_device(dev);
 
-	if (device_may_wakeup(&bdev->pdev->dev)) {
+	if (device_may_wakeup(dev)) {
 		error = enable_irq_wake(bdev->irq);
 		if (!error)
 			bt_dev_dbg(bdev, "BCM irq: enabled");
 	}
 
-unlock:
-	mutex_unlock(&bcm_device_lock);
-
 	return 0;
 }
 
 /* Platform resume callback */
 static int bcm_resume(struct device *dev)
 {
-	struct bcm_device *bdev = platform_get_drvdata(to_platform_device(dev));
-
-	bt_dev_dbg(bdev, "resume: is_suspended %d", bdev->is_suspended);
-
-	/* bcm_resume can be called at any time as long as platform device is
-	 * bound, so it should use bcm_device_lock to protect access to hci_uart
-	 * and device_wake-up GPIO.
-	 */
-	mutex_lock(&bcm_device_lock);
+	struct bcm_device *bdev =
+		serdev_device_get_drvdata(to_serdev_device(dev));
 
-	if (!bdev->hu)
-		goto unlock;
+	bt_dev_dbg(bdev, "resume");
 
-	if (device_may_wakeup(&bdev->pdev->dev)) {
+	if (device_may_wakeup(dev)) {
 		disable_irq_wake(bdev->irq);
 		bt_dev_dbg(bdev, "BCM irq: disabled");
 	}
 
 	bcm_resume_device(dev);
 
-unlock:
-	mutex_unlock(&bcm_device_lock);
-
 	pm_runtime_disable(dev);
 	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);
@@ -756,86 +626,80 @@ static int bcm_resource(struct acpi_resource *ares, void *data)
 }
 #endif /* CONFIG_ACPI */
 
-static int bcm_platform_probe(struct bcm_device *dev)
+static int bcm_get_resources(struct bcm_device *bdev)
 {
-	struct platform_device *pdev = dev->pdev;
-
-	dev->name = dev_name(&pdev->dev);
+	struct device *dev = &bdev->hu.serdev->dev;
 
-	dev->clk = devm_clk_get(&pdev->dev, NULL);
+	bdev->name = dev_name(dev);
+	bdev->clk = devm_clk_get(dev, NULL);
 
-	dev->device_wakeup = devm_gpiod_get_optional(&pdev->dev,
-						     "device-wakeup",
-						     GPIOD_OUT_LOW);
-	if (IS_ERR(dev->device_wakeup))
-		return PTR_ERR(dev->device_wakeup);
+	bdev->device_wakeup = devm_gpiod_get_optional(dev,
+						      "device-wakeup",
+						      GPIOD_OUT_LOW);
+	if (IS_ERR(bdev->device_wakeup))
+		return PTR_ERR(bdev->device_wakeup);
 
-	dev->shutdown = devm_gpiod_get_optional(&pdev->dev, "shutdown",
-						GPIOD_OUT_LOW);
-	if (IS_ERR(dev->shutdown))
-		return PTR_ERR(dev->shutdown);
+	bdev->shutdown = devm_gpiod_get_optional(dev, "shutdown",
+						 GPIOD_OUT_LOW);
+	if (IS_ERR(bdev->shutdown))
+		return PTR_ERR(bdev->shutdown);
 
 	/* IRQ can be declared in ACPI table as Interrupt or GpioInt */
-	dev->irq = platform_get_irq(pdev, 0);
-	if (dev->irq <= 0) {
+	if (bdev->irq == 0) {
 		struct gpio_desc *gpio;
 
-		gpio = devm_gpiod_get_optional(&pdev->dev, "host-wakeup",
+		gpio = devm_gpiod_get_optional(dev, "host-wakeup",
 					       GPIOD_IN);
 		if (IS_ERR(gpio))
 			return PTR_ERR(gpio);
 
-		dev->irq = gpiod_to_irq(gpio);
+		bdev->irq = gpiod_to_irq(gpio);
 	}
 
-	dev_info(&pdev->dev, "BCM irq: %d\n", dev->irq);
-
-	/* Make sure at-least one of the GPIO is defined and that
-	 * a name is specified for this instance
-	 */
-	if ((!dev->device_wakeup && !dev->shutdown) || !dev->name) {
-		dev_err(&pdev->dev, "invalid platform data\n");
-		return -EINVAL;
-	}
+	dev_info(dev, "BCM irq: %d\n", bdev->irq);
 
 	return 0;
 }
 
 #ifdef CONFIG_ACPI
-static int bcm_acpi_probe(struct bcm_device *dev)
+static int bcm_acpi_probe(struct bcm_device *bdev)
 {
-	struct platform_device *pdev = dev->pdev;
 	LIST_HEAD(resources);
 	const struct dmi_system_id *dmi_id;
 	const struct acpi_gpio_mapping *gpio_mapping = acpi_bcm_int_last_gpios;
+	struct device *dev = &bdev->hu.serdev->dev;
 	const struct acpi_device_id *id;
+	struct resource_entry *entry;
 	int ret;
 
 	/* Retrieve GPIO data */
-	id = acpi_match_device(pdev->dev.driver->acpi_match_table, &pdev->dev);
+	id = acpi_match_device(dev->driver->acpi_match_table, dev);
 	if (id)
 		gpio_mapping = (const struct acpi_gpio_mapping *) id->driver_data;
 
-	ret = devm_acpi_dev_add_driver_gpios(&pdev->dev, gpio_mapping);
-	if (ret)
-		return ret;
-
-	ret = bcm_platform_probe(dev);
+	ret = devm_acpi_dev_add_driver_gpios(dev, gpio_mapping);
 	if (ret)
 		return ret;
 
 	/* Retrieve UART ACPI info */
-	ret = acpi_dev_get_resources(ACPI_COMPANION(&dev->pdev->dev),
-				     &resources, bcm_resource, dev);
+	ret = acpi_dev_get_resources(ACPI_COMPANION(dev), &resources,
+				     bcm_resource, bdev);
 	if (ret < 0)
 		return ret;
+
+	resource_list_for_each_entry(entry, &resources) {
+		if (resource_type(entry->res) == IORESOURCE_IRQ) {
+			bdev->irq = entry->res->start;
+			break;
+		}
+	}
 	acpi_dev_free_resource_list(&resources);
 
 	dmi_id = dmi_first_match(bcm_active_low_irq_dmi_table);
 	if (dmi_id) {
-		bt_dev_warn(dev, "%s: Overwriting IRQ polarity to active low",
+		bt_dev_warn(bdev, "%s: Overwriting IRQ polarity to active low",
 			    dmi_id->ident);
-		dev->irq_active_low = true;
+		bdev->irq_active_low = true;
 	}
 
 	return 0;
@@ -847,48 +711,11 @@ static int bcm_acpi_probe(struct bcm_device *dev)
 }
 #endif /* CONFIG_ACPI */
 
-static int bcm_probe(struct platform_device *pdev)
+static int bcm_of_probe(struct bcm_device *bdev)
 {
-	struct bcm_device *dev;
-	int ret;
-
-	dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
-	if (!dev)
-		return -ENOMEM;
-
-	dev->pdev = pdev;
-
-	if (has_acpi_companion(&pdev->dev))
-		ret = bcm_acpi_probe(dev);
-	else
-		ret = bcm_platform_probe(dev);
-	if (ret)
-		return ret;
-
-	platform_set_drvdata(pdev, dev);
-
-	dev_info(&pdev->dev, "%s device registered.\n", dev->name);
-
-	/* Place this instance on the device list */
-	mutex_lock(&bcm_device_lock);
-	list_add_tail(&dev->list, &bcm_device_list);
-	mutex_unlock(&bcm_device_lock);
-
-	bcm_gpio_set_power(dev, false);
-
-	return 0;
-}
-
-static int bcm_remove(struct platform_device *pdev)
-{
-	struct bcm_device *dev = platform_get_drvdata(pdev);
-
-	mutex_lock(&bcm_device_lock);
-	list_del(&dev->list);
-	mutex_unlock(&bcm_device_lock);
-
-	dev_info(&pdev->dev, "%s device unregistered.\n", dev->name);
+	struct device *dev = &bdev->hu.serdev->dev;
 
+	device_property_read_u32(dev, "max-speed", &bdev->oper_speed);
 	return 0;
 }
 
@@ -907,6 +734,41 @@ static const struct hci_uart_proto bcm_proto = {
 	.dequeue	= bcm_dequeue,
 };
 
+static int bcm_probe(struct serdev_device *serdev)
+{
+	struct bcm_device *bdev;
+	int ret;
+
+	bdev = devm_kzalloc(&serdev->dev, sizeof(*bdev), GFP_KERNEL);
+	if (!bdev)
+		return -ENOMEM;
+
+	bdev->hu.serdev = serdev;
+	serdev_device_set_drvdata(serdev, bdev);
+
+	if (has_acpi_companion(&serdev->dev))
+		ret = bcm_acpi_probe(bdev);
+	else
+		ret = bcm_of_probe(bdev);
+	if (ret)
+		return ret;
+
+	ret = bcm_get_resources(bdev);
+	if (ret)
+		return ret;
+
+	bcm_gpio_set_power(bdev, false);
+
+	return hci_uart_register_device(&bdev->hu, &bcm_proto);
+}
+
+static void bcm_remove(struct serdev_device *serdev)
+{
+	struct bcm_device *bdev = serdev_device_get_drvdata(serdev);
+
+	hci_uart_unregister_device(&bdev->hu);
+}
+
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id bcm_acpi_match[] = {
 	{ "BCM2E1A", (kernel_ulong_t)&acpi_bcm_int_last_gpios },
@@ -931,72 +793,33 @@ static const struct acpi_device_id bcm_acpi_match[] = {
 MODULE_DEVICE_TABLE(acpi, bcm_acpi_match);
 #endif
 
+#ifdef CONFIG_OF
+static const struct of_device_id bcm_bluetooth_of_match[] = {
+	{ .compatible = "brcm,bcm43438-bt" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, bcm_bluetooth_of_match);
+#endif
+
 /* Platform suspend and resume callbacks */
 static const struct dev_pm_ops bcm_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(bcm_suspend, bcm_resume)
 	SET_RUNTIME_PM_OPS(bcm_suspend_device, bcm_resume_device, NULL)
 };
 
-static struct platform_driver bcm_driver = {
+static struct serdev_device_driver bcm_serdev_driver = {
 	.probe = bcm_probe,
 	.remove = bcm_remove,
 	.driver = {
 		.name = "hci_bcm",
-		.acpi_match_table = ACPI_PTR(bcm_acpi_match),
 		.pm = &bcm_pm_ops,
-	},
-};
-
-static int bcm_serdev_probe(struct serdev_device *serdev)
-{
-	struct bcm_serdev *bcmdev;
-	u32 speed;
-	int err;
-
-	bcmdev = devm_kzalloc(&serdev->dev, sizeof(*bcmdev), GFP_KERNEL);
-	if (!bcmdev)
-		return -ENOMEM;
-
-	bcmdev->hu.serdev = serdev;
-	serdev_device_set_drvdata(serdev, bcmdev);
-
-	err = device_property_read_u32(&serdev->dev, "max-speed", &speed);
-	if (!err)
-		bcmdev->hu.oper_speed = speed;
-
-	return hci_uart_register_device(&bcmdev->hu, &bcm_proto);
-}
-
-static void bcm_serdev_remove(struct serdev_device *serdev)
-{
-	struct bcm_serdev *bcmdev = serdev_device_get_drvdata(serdev);
-
-	hci_uart_unregister_device(&bcmdev->hu);
-}
-
-#ifdef CONFIG_OF
-static const struct of_device_id bcm_bluetooth_of_match[] = {
-	{ .compatible = "brcm,bcm43438-bt" },
-	{ },
-};
-MODULE_DEVICE_TABLE(of, bcm_bluetooth_of_match);
-#endif
-
-static struct serdev_device_driver bcm_serdev_driver = {
-	.probe = bcm_serdev_probe,
-	.remove = bcm_serdev_remove,
-	.driver = {
-		.name = "hci_uart_bcm",
 		.of_match_table = of_match_ptr(bcm_bluetooth_of_match),
+		.acpi_match_table = ACPI_PTR(bcm_acpi_match),
 	},
 };
 
 int __init bcm_init(void)
 {
-	/* For now, we need to keep both platform device
-	 * driver (ACPI generated) and serdev driver (DT).
-	 */
-	platform_driver_register(&bcm_driver);
 	serdev_device_driver_register(&bcm_serdev_driver);
 
 	return hci_uart_register_proto(&bcm_proto);
@@ -1004,7 +827,6 @@ int __init bcm_init(void)
 
 int __exit bcm_deinit(void)
 {
-	platform_driver_unregister(&bcm_driver);
 	serdev_device_driver_unregister(&bcm_serdev_driver);
 
 	return hci_uart_unregister_proto(&bcm_proto);
-- 
2.14.2


^ permalink raw reply related

* Re: [PATCH 1/2] serial: sh-sci: Support for variable HSCIF hardware RX timeout
From: Geert Uytterhoeven @ 2017-10-02  7:30 UTC (permalink / raw)
  To: Ulrich Hecht
  Cc: Linux-Renesas, Wolfram Sang, linux-serial@vger.kernel.org,
	Magnus Damm
In-Reply-To: <1506690534-27302-2-git-send-email-ulrich.hecht+renesas@gmail.com>

Hi Uli,

On Fri, Sep 29, 2017 at 3:08 PM, Ulrich Hecht
<ulrich.hecht+renesas@gmail.com> wrote:
> HSCIF has facilities that allow changing the timeout after which an RX
> interrupt is triggered even if the FIFO is not filled. This patch allows
> changing the default (15 bits of silence) using the existing sysfs
> attribute "rx_fifo_timeout".

Thanks for the update!

> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH 2/2] serial: sh-sci: Support for HSCIF RX sampling point adjustment
From: Geert Uytterhoeven @ 2017-10-02  8:05 UTC (permalink / raw)
  To: Ulrich Hecht
  Cc: Linux-Renesas, Wolfram Sang, linux-serial@vger.kernel.org,
	Magnus Damm
In-Reply-To: <1506690534-27302-3-git-send-email-ulrich.hecht+renesas@gmail.com>

Hi Uli,

On Fri, Sep 29, 2017 at 3:08 PM, Ulrich Hecht
<ulrich.hecht+renesas@gmail.com> wrote:
> HSCIF has facilities that allow moving the RX sampling point by between
> -8 and 7 sampling cycles (one sampling cycles equals 1/15 of a bit
> by default) to improve the error margin in case of slightly mismatched
> bit rates between sender and receiver.
>
> This patch allows changing the default (0, meaning center) using the
> sysfs attribute "rx_sampling_point".
>
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>

Thanks for your patch!

Have you noticed any improvements by fiddling with the sampling point?

While this patch is useful as an investigation tool due to the sysfs
interface, I don't think it should be applied as-is.

I guess the goal is to select automatically the optimal sampling point,
based on requested bit rate and available clock input rates, right?

Or perhaps you want to keep the sysfs interface, to accommodate a sender
that uses an off bit rate, unbeknownst to Linux (and unsupported by Linux,
as software can request standard bit rates only) and thus needs user
configuration?
If that is the case, perhaps the sysfs interface should not be provided the
raw sampling point offset, but the actual bit rate used by the other side,
so the driver can calculate the offset?

> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -153,6 +153,7 @@ struct sci_port {
>         struct timer_list               rx_fifo_timer;
>         int                             rx_fifo_timeout;
>         u16                             hscif_tot;
> +       u16                             hscif_srhp;

s16, or even s8?

>
>         bool has_rtscts;
>         bool autorts;
> @@ -992,6 +993,42 @@ static int sci_handle_breaks(struct uart_port *port)
>         return copied;
>  }
>
> +static ssize_t rx_sampling_point_show(struct device *dev,
> +                              struct device_attribute *attr,
> +                              char *buf)
> +{
> +       struct uart_port *port = dev_get_drvdata(dev);
> +       struct sci_port *sci = to_sci_port(port);
> +
> +       return sprintf(buf, "%d\n", sci->hscif_srhp);

... else it will not output negative numbers here.

> +}
> +
> +static ssize_t rx_sampling_point_store(struct device *dev,
> +                               struct device_attribute *attr,
> +                               const char *buf,
> +                               size_t count)
> +{
> +       struct uart_port *port = dev_get_drvdata(dev);
> +       struct sci_port *sci = to_sci_port(port);
> +       int ret;
> +       long r;

s8 or just int?

> +
> +       ret = kstrtol(buf, 0, &r);

kstrtos8() or kstrtoint()?

> +       if (ret)
> +               return ret;
> +
> +       if (r < -8 || r > 7)
> +               return -EINVAL;
> +
> +       sci->hscif_srhp = r;
> +
> +       return count;
> +}
> +
> +static DEVICE_ATTR(rx_sampling_point, 0644,
> +                  rx_sampling_point_show,
> +                  rx_sampling_point_store);
> +
>  static int scif_set_rtrg(struct uart_port *port, int rx_trig)
>  {
>         unsigned int bits;
> @@ -2378,8 +2415,6 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
>                 serial_port_out(port, SCSCR, scr_val | s->hscif_tot);
>                 serial_port_out(port, SCSMR, smr_val);
>                 serial_port_out(port, SCBRR, brr);
> -               if (sci_getreg(port, HSSRR)->size)
> -                       serial_port_out(port, HSSRR, srr | HSCIF_SRE);

Why have you moved the setting of HSSRR?
AFAIK all bit rate settings should be completed before the one bit delay below.

>                 /* Wait one bit interval */
>                 udelay((1000000 + (baud - 1)) / baud);
> @@ -2393,6 +2428,18 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
>                 serial_port_out(port, SCSMR, smr_val);
>         }
>
> +       if (sci_getreg(port, HSSRR)->size) {
> +               u16 hssrr = srr | HSCIF_SRE;
> +
> +               if (s->hscif_srhp) {
> +                       u16 srhp = (s->hscif_srhp << HSCIF_SRHP_SHIFT) &
> +                                  HSCIF_SRHP_MASK;
> +
> +                       hssrr |= HSCIF_SRDE | srhp;
> +               }
> +               serial_port_out(port, HSSRR, hssrr);
> +       }
> +
>         sci_init_pins(port, termios->c_cflag);
>
>         port->status &= ~UPSTAT_AUTOCTS;

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH] serial: imx-serial - move DMA buffer configuration to DT
From: Han, Nandor (GE Healthcare) @ 2017-10-02 13:17 UTC (permalink / raw)
  To: Uwe Kleine-König, Romain Perier
  Cc: Greg Kroah-Hartman, Jiri Slaby,
	linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20170629182618.jpahpmuq364ldcv2-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

> -----Original Message-----
> From: Uwe Kleine-König [mailto:u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org]
> Sent: 29 June 2017 21:26
> To: Romain Perier <romain.perier-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
> Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>; Jiri Slaby
> <jslaby-IBi9RG/b67k@public.gmane.org>; linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Han, Nandor (GE
> Healthcare) <nandor.han-JJi787mZWgc@public.gmane.org>; devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: EXT: Re: [PATCH] serial: imx-serial - move DMA buffer configuration
> to DT
> 
> Hello,
> 
> Cc: += devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> 
> On Wed, Jun 28, 2017 at 12:15:14PM +0200, Romain Perier wrote:
> > From: Nandor Han <nandor.han-JJi787mZWgc@public.gmane.org>
> >
> > The size of the DMA buffer can affect the delta time between data
> > being produced and data being consumed. Basically the DMA system will
> > move data to tty buffer when a) DMA buffer is full b) serial line is idle.
> > The situation is visible when producer generates data continuously and
> > there is no possibility for idle line. At this point the DMA buffer is
> > directly affecting the delta time.

Hi Uwe,
   Maybe I can explain a bit better the situation. At least I've tried to explain well enough
the problem and the fix. :)

> 
> This doesn't look like a hw property but a configuration item. Also I don't
> understand the problematic case. The i.MX is sending continuously and then
> doesn't receive bytes until the DMA buffer is full?

Yes

> What is the DMA buffer?
> You don't mean the FIFO here, do you?
> 

DMA buffer is not HW FIFO. Is the buffer provided by serial driver to DMA to store data.

> That doesn't sound like a good fix but more like a work around. Which other
> options did you test to fix your problem?
> 

I haven't tried any other, because except using maybe, ioctl I haven't got anything better.

Our problem is that in our system some serial ports needs to have really low data latency, where others trade more bytes over data latency. This situation results in a need of beeing able to have different DMA buffer size for different ports. 

How can DMA buffer size affect latency?
DMA works like this: (To answer to your question DMA buffer is not FIFO)
 1. Transfer the data from HW FIFO to DMA buffer based on some interrupts (character received, etc)
 2. Transfer the DMA buffer back to serial port based on some events (buffer full, aging timer, etc)
 3. Serial port forwards to tty buffer.

Data availability to consumer depends on: DMA buffer size, baud rate and communication pattern. By communication patter I'm refering that we send data continuoselly (serial line is never idle) or packet by packet (serial line is idle in between)
Example:
      Baud: 19200 (1Byte = 0.52 ms)
      DMA buffer size: 100 bytes
      Communication pattern: continuously 
      =>  DMA will return data to serial port only when DMA buffer is full, since the communication is continuously. This result in a data latency of 0.52 ms* 100bytes = 52ms. In case the buffer will be 200bytes the letency will be double.

I agree with you, this is not directly a hw property but a DMA configuration item. 
But I've found this to be the best way to configure this comparing with using ioctl.

Let me know if you need more clarification and I would really be open to other options that will solve our problem.

<snip>

> > +- fsl,dma-size : Indicate the size of the DMA buffer and its periods
> 
> This is a sparse description, just from reading that I don't understand what it
> does.
> 

Serial driver configures a circular ring of buffers for DMA. Here we can configure the size and the number of buffers.

> > +/* RX DMA buffer periods */
> > +#define RX_DMA_PERIODS 4
> > +#define RX_BUF_SIZE	(PAGE_SIZE)
> 
> These names should include "DEFAULT" in their name now to fit their new
> semantic.
> 

Agree. 


Regards,
    Nandor
--
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

^ permalink raw reply

* [PATCH] tty: serial: jsm: Add space before the open brace
From: Gimcuan Hui @ 2017-10-02 14:48 UTC (permalink / raw)
  To: Guilherme G. Piccoli, Greg Kroah-Hartman, Jiri Slaby,
	linux-serial, linux-kernel
  Cc: Gimcuan Hui

This patch fixes the checkpatch.pl error complain:

ERROR: space required before the open brace '{'

Signed-off-by: Gimcuan Hui <gimcuan@gmail.com>
---
 drivers/tty/serial/jsm/jsm_tty.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/jsm/jsm_tty.c b/drivers/tty/serial/jsm/jsm_tty.c
index ec7d838..e69227c 100644
--- a/drivers/tty/serial/jsm/jsm_tty.c
+++ b/drivers/tty/serial/jsm/jsm_tty.c
@@ -474,7 +474,7 @@ int jsm_uart_port_init(struct jsm_board *brd)
 			set_bit(line, linemap);
 		brd->channels[i]->uart_port.line = line;
 		rc = uart_add_one_port (&jsm_uart_driver, &brd->channels[i]->uart_port);
-		if (rc){
+		if (rc) {
 			printk(KERN_INFO "jsm: Port %d failed. Aborting...\n", i);
 			return rc;
 		}
-- 
2.7.4

^ permalink raw reply related

* [PATCH 0/9] Bluetooth: hci_bcm: Add (runtime)pm support to the serdev drv
From: Hans de Goede @ 2017-10-02 15:23 UTC (permalink / raw)
  To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg
  Cc: Hans de Goede, Frédéric Danis, Sebastian Reichel,
	robh-DgEjT+Ai2ygdnm+yROfE0A,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA

Hi All,

The ACPI subsys is going to move over to instantiating ACPI enumerated
HCIs as serdevs, rather then as platform devices.

This means that the serdev driver paths of hci_bcm.c also need to start
supporting (runtime)pm through GPIOs and a host-wake IRQ.

This series makes most of the code in hci_bcm.c indepdenent from how
the hci was instantiated and then more or less unifies the platform
and serdev drivers, adding (runtime)pm / GPIO / IRQ support to the
serdev driver.

Besides adding (runtime)pm support, GPIO control in general is necessary
to e.g. keep the ACPI BCM2E7E id used with the BCM4356A2 wifi/bt combo
on the GPD pocket working after moving over to serdev enumeration.

I've chosen to keep the platform enumeration based code paths in place
for now. This way we can first merge this series and then, once this
series is in place, the first 2 patches of Frédéric Danis RFC series
for ACPI serdev support can be merged without causing any regressions.

Once the ACPI serdev support is merged a follow up patch removing the
platform-device related code can be submitted.

Note the 2nd patch in this series is a resend of a bug-fix I submitted
a couple of days ago, I've included that with the series to avoid
conflicts when applying the series without that fix.

Regards,

Hans

^ permalink raw reply

* [PATCH 1/9] Bluetooth: hci_uart_set_flow_control: Fix NULL deref when using serdev
From: Hans de Goede @ 2017-10-02 15:23 UTC (permalink / raw)
  To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg
  Cc: Hans de Goede, Frédéric Danis, Sebastian Reichel, robh,
	linux-bluetooth, linux-serial, linux-acpi
In-Reply-To: <20171002152356.4714-1-hdegoede@redhat.com>

Fix a NULL pointer deref (hu->tty) when calling hci_uart_set_flow_control
on hci_uart-s using serdev.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/bluetooth/hci_ldisc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index a746627e784e..d02f6d029093 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -41,6 +41,7 @@
 #include <linux/ioctl.h>
 #include <linux/skbuff.h>
 #include <linux/firmware.h>
+#include <linux/serdev.h>
 
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>
@@ -298,6 +299,11 @@ void hci_uart_set_flow_control(struct hci_uart *hu, bool enable)
 	unsigned int set = 0;
 	unsigned int clear = 0;
 
+	if (hu->serdev) {
+		serdev_device_set_flow_control(hu->serdev, !enable);
+		return;
+	}
+
 	if (enable) {
 		/* Disable hardware flow control */
 		ktermios = tty->termios;
-- 
2.14.2


^ permalink raw reply related

* [PATCH 2/9] Bluetooth: hci_bcm: Fix setting of irq trigger type
From: Hans de Goede @ 2017-10-02 15:23 UTC (permalink / raw)
  To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg
  Cc: Hans de Goede, Frédéric Danis, Sebastian Reichel, robh,
	linux-bluetooth, linux-serial, linux-acpi
In-Reply-To: <20171002152356.4714-1-hdegoede@redhat.com>

This commit fixes 2 issues with host-wake irq trigger type handling
in hci_bcm:

1) bcm_setup_sleep sets sleep_params.host_wake_active based on
bcm_device.irq_polarity, but bcm_request_irq was always requesting
IRQF_TRIGGER_RISING as trigger type independent of irq_polarity.

This was a problem when the irq is described as a GpioInt rather then
an Interrupt in the DSDT as for GpioInt-s the value passed to request_irq
is honored. This commit fixes this by requesting the correct trigger
type depending on bcm_device.irq_polarity.

2) bcm_device.irq_polarity was used to directly store an ACPI polarity
value (ACPI_ACTIVE_*). This is undesirable because hci_bcm is also
used with device-tree and checking for something like ACPI_ACTIVE_LOW
in a non ACPI specific function like bcm_request_irq feels wrong.

This commit fixes this by renaming irq_polarity to irq_active_low
and changing its type to a bool.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/bluetooth/hci_bcm.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index c821234b9668..2285ca673ae3 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -68,7 +68,7 @@ struct bcm_device {
 	u32			init_speed;
 	u32			oper_speed;
 	int			irq;
-	u8			irq_polarity;
+	bool			irq_active_low;
 
 #ifdef CONFIG_PM
 	struct hci_uart		*hu;
@@ -213,7 +213,9 @@ static int bcm_request_irq(struct bcm_data *bcm)
 	}
 
 	err = devm_request_irq(&bdev->pdev->dev, bdev->irq, bcm_host_wake,
-			       IRQF_TRIGGER_RISING, "host_wake", bdev);
+			       bdev->irq_active_low ? IRQF_TRIGGER_FALLING :
+						      IRQF_TRIGGER_RISING,
+			       "host_wake", bdev);
 	if (err)
 		goto unlock;
 
@@ -253,7 +255,7 @@ static int bcm_setup_sleep(struct hci_uart *hu)
 	struct sk_buff *skb;
 	struct bcm_set_sleep_mode sleep_params = default_sleep_params;
 
-	sleep_params.host_wake_active = !bcm->dev->irq_polarity;
+	sleep_params.host_wake_active = !bcm->dev->irq_active_low;
 
 	skb = __hci_cmd_sync(hu->hdev, 0xfc27, sizeof(sleep_params),
 			     &sleep_params, HCI_INIT_TIMEOUT);
@@ -690,10 +692,8 @@ static const struct acpi_gpio_mapping acpi_bcm_int_first_gpios[] = {
 };
 
 #ifdef CONFIG_ACPI
-static u8 acpi_active_low = ACPI_ACTIVE_LOW;
-
 /* IRQ polarity of some chipsets are not defined correctly in ACPI table. */
-static const struct dmi_system_id bcm_wrong_irq_dmi_table[] = {
+static const struct dmi_system_id bcm_active_low_irq_dmi_table[] = {
 	{
 		.ident = "Asus T100TA",
 		.matches = {
@@ -701,7 +701,6 @@ static const struct dmi_system_id bcm_wrong_irq_dmi_table[] = {
 					"ASUSTeK COMPUTER INC."),
 			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "T100TA"),
 		},
-		.driver_data = &acpi_active_low,
 	},
 	{
 		.ident = "Asus T100CHI",
@@ -710,7 +709,6 @@ static const struct dmi_system_id bcm_wrong_irq_dmi_table[] = {
 					"ASUSTeK COMPUTER INC."),
 			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "T100CHI"),
 		},
-		.driver_data = &acpi_active_low,
 	},
 	{	/* Handle ThinkPad 8 tablets with BCM2E55 chipset ACPI ID */
 		.ident = "Lenovo ThinkPad 8",
@@ -718,7 +716,6 @@ static const struct dmi_system_id bcm_wrong_irq_dmi_table[] = {
 			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "LENOVO"),
 			DMI_EXACT_MATCH(DMI_PRODUCT_VERSION, "ThinkPad 8"),
 		},
-		.driver_data = &acpi_active_low,
 	},
 	{ }
 };
@@ -733,13 +730,13 @@ static int bcm_resource(struct acpi_resource *ares, void *data)
 	switch (ares->type) {
 	case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
 		irq = &ares->data.extended_irq;
-		dev->irq_polarity = irq->polarity;
+		dev->irq_active_low = irq->polarity == ACPI_ACTIVE_LOW;
 		break;
 
 	case ACPI_RESOURCE_TYPE_GPIO:
 		gpio = &ares->data.gpio;
 		if (gpio->connection_type == ACPI_RESOURCE_GPIO_TYPE_INT)
-			dev->irq_polarity = gpio->polarity;
+			dev->irq_active_low = gpio->polarity == ACPI_ACTIVE_LOW;
 		break;
 
 	case ACPI_RESOURCE_TYPE_SERIAL_BUS:
@@ -834,11 +831,11 @@ static int bcm_acpi_probe(struct bcm_device *dev)
 		return ret;
 	acpi_dev_free_resource_list(&resources);
 
-	dmi_id = dmi_first_match(bcm_wrong_irq_dmi_table);
+	dmi_id = dmi_first_match(bcm_active_low_irq_dmi_table);
 	if (dmi_id) {
 		bt_dev_warn(dev, "%s: Overwriting IRQ polarity to active low",
 			    dmi_id->ident);
-		dev->irq_polarity = *(u8 *)dmi_id->driver_data;
+		dev->irq_active_low = true;
 	}
 
 	return 0;
-- 
2.14.2


^ permalink raw reply related

* [PATCH 3/9] Bluetooth: hci_bcm: Move bcm_platform_probe call out of bcm_acpi_probe
From: Hans de Goede @ 2017-10-02 15:23 UTC (permalink / raw)
  To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg
  Cc: Hans de Goede, Frédéric Danis, Sebastian Reichel, robh,
	linux-bluetooth, linux-serial, linux-acpi
In-Reply-To: <20171002152356.4714-1-hdegoede@redhat.com>

Since bcm_acpi_probe calls bcm_platform_probe, bcm_probe always ends up
calling bcm_platform_probe.

This commit simplifies things by making bcm_probe always call
bcm_platform_probe itself.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/bluetooth/hci_bcm.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 2285ca673ae3..00103307a776 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -820,10 +820,6 @@ static int bcm_acpi_probe(struct bcm_device *dev)
 	if (ret)
 		return ret;
 
-	ret = bcm_platform_probe(dev);
-	if (ret)
-		return ret;
-
 	/* Retrieve UART ACPI info */
 	ret = acpi_dev_get_resources(ACPI_COMPANION(&dev->pdev->dev),
 				     &resources, bcm_resource, dev);
@@ -858,10 +854,13 @@ static int bcm_probe(struct platform_device *pdev)
 
 	dev->pdev = pdev;
 
-	if (has_acpi_companion(&pdev->dev))
+	if (has_acpi_companion(&pdev->dev)) {
 		ret = bcm_acpi_probe(dev);
-	else
-		ret = bcm_platform_probe(dev);
+		if (ret)
+			return ret;
+	}
+
+	ret = bcm_platform_probe(dev);
 	if (ret)
 		return ret;
 
-- 
2.14.2


^ permalink raw reply related

* [PATCH 4/9] Bluetooth: hci_bcm: Move platform_get_irq call to bcm_probe
From: Hans de Goede @ 2017-10-02 15:23 UTC (permalink / raw)
  To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg
  Cc: Hans de Goede, Frédéric Danis, Sebastian Reichel, robh,
	linux-bluetooth, linux-serial, linux-acpi
In-Reply-To: <20171002152356.4714-1-hdegoede@redhat.com>

The ACPI subsys is going to move over to instantiating ACPI enumerated
HCIs as serdevs, rather then as platform devices.

Most of the code in bcm_platform_probe is actually not platform
specific and will work with any struct device passed to it, the one
platform specific call in bcm_platform_probe is platform_get_irq.

This commit moves platform_get_irq call to the platform-driver's bcm_probe
function, this is a preparation patch for adding (runtime)pm support to
the serdev path.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/bluetooth/hci_bcm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 00103307a776..ea530a56778d 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -776,7 +776,6 @@ static int bcm_platform_probe(struct bcm_device *dev)
 		return PTR_ERR(dev->shutdown);
 
 	/* IRQ can be declared in ACPI table as Interrupt or GpioInt */
-	dev->irq = platform_get_irq(pdev, 0);
 	if (dev->irq <= 0) {
 		struct gpio_desc *gpio;
 
@@ -853,6 +852,7 @@ static int bcm_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	dev->pdev = pdev;
+	dev->irq = platform_get_irq(pdev, 0);
 
 	if (has_acpi_companion(&pdev->dev)) {
 		ret = bcm_acpi_probe(dev);
-- 
2.14.2


^ permalink raw reply related

* [PATCH 5/9] Bluetooth: hci_bcm: Store device pointer instead of platform_device pointer
From: Hans de Goede @ 2017-10-02 15:23 UTC (permalink / raw)
  To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg
  Cc: Hans de Goede, Frédéric Danis, Sebastian Reichel,
	robh-DgEjT+Ai2ygdnm+yROfE0A,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171002152356.4714-1-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

The ACPI subsys is going to move over to instantiating ACPI enumerated
HCIs as serdevs, rather then as platform devices.

This means that the serdev driver paths of hci_bcm.c also need to start
supporting (runtime)pm through GPIOs and a host-wake IRQ.

The hci_bcm code is already mostly independent of how the HCI gets
instantiated, but even though the code only cares about pdev->dev, it
was storing pdev itself in struct bcm_device.

This commit stores pdev->dev rather then pdev in struct bcm_device, this
is a preparation patch for adding (runtime)pm support to the serdev path.

Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 drivers/bluetooth/hci_bcm.c | 73 ++++++++++++++++++++++-----------------------
 1 file changed, 35 insertions(+), 38 deletions(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index ea530a56778d..cfc87fb5051c 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -56,7 +56,7 @@
 struct bcm_device {
 	struct list_head	list;
 
-	struct platform_device	*pdev;
+	struct device		*dev;
 
 	const char		*name;
 	struct gpio_desc	*device_wakeup;
@@ -188,9 +188,9 @@ static irqreturn_t bcm_host_wake(int irq, void *data)
 
 	bt_dev_dbg(bdev, "Host wake IRQ");
 
-	pm_runtime_get(&bdev->pdev->dev);
-	pm_runtime_mark_last_busy(&bdev->pdev->dev);
-	pm_runtime_put_autosuspend(&bdev->pdev->dev);
+	pm_runtime_get(bdev->dev);
+	pm_runtime_mark_last_busy(bdev->dev);
+	pm_runtime_put_autosuspend(bdev->dev);
 
 	return IRQ_HANDLED;
 }
@@ -212,20 +212,20 @@ static int bcm_request_irq(struct bcm_data *bcm)
 		goto unlock;
 	}
 
-	err = devm_request_irq(&bdev->pdev->dev, bdev->irq, bcm_host_wake,
+	err = devm_request_irq(bdev->dev, bdev->irq, bcm_host_wake,
 			       bdev->irq_active_low ? IRQF_TRIGGER_FALLING :
 						      IRQF_TRIGGER_RISING,
 			       "host_wake", bdev);
 	if (err)
 		goto unlock;
 
-	device_init_wakeup(&bdev->pdev->dev, true);
+	device_init_wakeup(bdev->dev, true);
 
-	pm_runtime_set_autosuspend_delay(&bdev->pdev->dev,
+	pm_runtime_set_autosuspend_delay(bdev->dev,
 					 BCM_AUTOSUSPEND_DELAY);
-	pm_runtime_use_autosuspend(&bdev->pdev->dev);
-	pm_runtime_set_active(&bdev->pdev->dev);
-	pm_runtime_enable(&bdev->pdev->dev);
+	pm_runtime_use_autosuspend(bdev->dev);
+	pm_runtime_set_active(bdev->dev);
+	pm_runtime_enable(bdev->dev);
 
 unlock:
 	mutex_unlock(&bcm_device_lock);
@@ -332,7 +332,7 @@ static int bcm_open(struct hci_uart *hu)
 		 * platform device (saved during device probe) and
 		 * parent of tty device used by hci_uart
 		 */
-		if (hu->tty->dev->parent == dev->pdev->dev.parent) {
+		if (hu->tty->dev->parent == dev->dev->parent) {
 			bcm->dev = dev;
 			hu->init_speed = dev->init_speed;
 			hu->oper_speed = dev->oper_speed;
@@ -367,12 +367,12 @@ static int bcm_close(struct hci_uart *hu)
 	if (bcm_device_exists(bdev)) {
 		bcm_gpio_set_power(bdev, false);
 #ifdef CONFIG_PM
-		pm_runtime_disable(&bdev->pdev->dev);
-		pm_runtime_set_suspended(&bdev->pdev->dev);
+		pm_runtime_disable(bdev->dev);
+		pm_runtime_set_suspended(bdev->dev);
 
-		if (device_can_wakeup(&bdev->pdev->dev)) {
-			devm_free_irq(&bdev->pdev->dev, bdev->irq, bdev);
-			device_init_wakeup(&bdev->pdev->dev, false);
+		if (device_can_wakeup(bdev->dev)) {
+			devm_free_irq(bdev->dev, bdev->irq, bdev);
+			device_init_wakeup(bdev->dev, false);
 		}
 
 		bdev->hu = NULL;
@@ -506,9 +506,9 @@ static int bcm_recv(struct hci_uart *hu, const void *data, int count)
 		/* Delay auto-suspend when receiving completed packet */
 		mutex_lock(&bcm_device_lock);
 		if (bcm->dev && bcm_device_exists(bcm->dev)) {
-			pm_runtime_get(&bcm->dev->pdev->dev);
-			pm_runtime_mark_last_busy(&bcm->dev->pdev->dev);
-			pm_runtime_put_autosuspend(&bcm->dev->pdev->dev);
+			pm_runtime_get(bcm->dev->dev);
+			pm_runtime_mark_last_busy(bcm->dev->dev);
+			pm_runtime_put_autosuspend(bcm->dev->dev);
 		}
 		mutex_unlock(&bcm_device_lock);
 	}
@@ -539,15 +539,15 @@ static struct sk_buff *bcm_dequeue(struct hci_uart *hu)
 
 	if (bcm_device_exists(bcm->dev)) {
 		bdev = bcm->dev;
-		pm_runtime_get_sync(&bdev->pdev->dev);
+		pm_runtime_get_sync(bdev->dev);
 		/* Shall be resumed here */
 	}
 
 	skb = skb_dequeue(&bcm->txq);
 
 	if (bdev) {
-		pm_runtime_mark_last_busy(&bdev->pdev->dev);
-		pm_runtime_put_autosuspend(&bdev->pdev->dev);
+		pm_runtime_mark_last_busy(bdev->dev);
+		pm_runtime_put_autosuspend(bdev->dev);
 	}
 
 	mutex_unlock(&bcm_device_lock);
@@ -623,7 +623,7 @@ static int bcm_suspend(struct device *dev)
 	if (pm_runtime_active(dev))
 		bcm_suspend_device(dev);
 
-	if (device_may_wakeup(&bdev->pdev->dev)) {
+	if (device_may_wakeup(dev)) {
 		error = enable_irq_wake(bdev->irq);
 		if (!error)
 			bt_dev_dbg(bdev, "BCM irq: enabled");
@@ -651,7 +651,7 @@ static int bcm_resume(struct device *dev)
 	if (!bdev->hu)
 		goto unlock;
 
-	if (device_may_wakeup(&bdev->pdev->dev)) {
+	if (device_may_wakeup(dev)) {
 		disable_irq_wake(bdev->irq);
 		bt_dev_dbg(bdev, "BCM irq: disabled");
 	}
@@ -758,19 +758,17 @@ static int bcm_resource(struct acpi_resource *ares, void *data)
 
 static int bcm_platform_probe(struct bcm_device *dev)
 {
-	struct platform_device *pdev = dev->pdev;
+	dev->name = dev_name(dev->dev);
 
-	dev->name = dev_name(&pdev->dev);
+	dev->clk = devm_clk_get(dev->dev, NULL);
 
-	dev->clk = devm_clk_get(&pdev->dev, NULL);
-
-	dev->device_wakeup = devm_gpiod_get_optional(&pdev->dev,
+	dev->device_wakeup = devm_gpiod_get_optional(dev->dev,
 						     "device-wakeup",
 						     GPIOD_OUT_LOW);
 	if (IS_ERR(dev->device_wakeup))
 		return PTR_ERR(dev->device_wakeup);
 
-	dev->shutdown = devm_gpiod_get_optional(&pdev->dev, "shutdown",
+	dev->shutdown = devm_gpiod_get_optional(dev->dev, "shutdown",
 						GPIOD_OUT_LOW);
 	if (IS_ERR(dev->shutdown))
 		return PTR_ERR(dev->shutdown);
@@ -779,7 +777,7 @@ static int bcm_platform_probe(struct bcm_device *dev)
 	if (dev->irq <= 0) {
 		struct gpio_desc *gpio;
 
-		gpio = devm_gpiod_get_optional(&pdev->dev, "host-wakeup",
+		gpio = devm_gpiod_get_optional(dev->dev, "host-wakeup",
 					       GPIOD_IN);
 		if (IS_ERR(gpio))
 			return PTR_ERR(gpio);
@@ -787,13 +785,13 @@ static int bcm_platform_probe(struct bcm_device *dev)
 		dev->irq = gpiod_to_irq(gpio);
 	}
 
-	dev_info(&pdev->dev, "BCM irq: %d\n", dev->irq);
+	dev_info(dev->dev, "BCM irq: %d\n", dev->irq);
 
 	/* Make sure at-least one of the GPIO is defined and that
 	 * a name is specified for this instance
 	 */
 	if ((!dev->device_wakeup && !dev->shutdown) || !dev->name) {
-		dev_err(&pdev->dev, "invalid platform data\n");
+		dev_err(dev->dev, "invalid platform data\n");
 		return -EINVAL;
 	}
 
@@ -803,7 +801,6 @@ static int bcm_platform_probe(struct bcm_device *dev)
 #ifdef CONFIG_ACPI
 static int bcm_acpi_probe(struct bcm_device *dev)
 {
-	struct platform_device *pdev = dev->pdev;
 	LIST_HEAD(resources);
 	const struct dmi_system_id *dmi_id;
 	const struct acpi_gpio_mapping *gpio_mapping = acpi_bcm_int_last_gpios;
@@ -811,16 +808,16 @@ static int bcm_acpi_probe(struct bcm_device *dev)
 	int ret;
 
 	/* Retrieve GPIO data */
-	id = acpi_match_device(pdev->dev.driver->acpi_match_table, &pdev->dev);
+	id = acpi_match_device(dev->dev->driver->acpi_match_table, dev->dev);
 	if (id)
 		gpio_mapping = (const struct acpi_gpio_mapping *) id->driver_data;
 
-	ret = devm_acpi_dev_add_driver_gpios(&pdev->dev, gpio_mapping);
+	ret = devm_acpi_dev_add_driver_gpios(dev->dev, gpio_mapping);
 	if (ret)
 		return ret;
 
 	/* Retrieve UART ACPI info */
-	ret = acpi_dev_get_resources(ACPI_COMPANION(&dev->pdev->dev),
+	ret = acpi_dev_get_resources(ACPI_COMPANION(dev->dev),
 				     &resources, bcm_resource, dev);
 	if (ret < 0)
 		return ret;
@@ -851,7 +848,7 @@ static int bcm_probe(struct platform_device *pdev)
 	if (!dev)
 		return -ENOMEM;
 
-	dev->pdev = pdev;
+	dev->dev = &pdev->dev;
 	dev->irq = platform_get_irq(pdev, 0);
 
 	if (has_acpi_companion(&pdev->dev)) {
-- 
2.14.2

^ permalink raw reply related

* [PATCH 6/9] Bluetooth: hci_bcm: Rename bcm_platform_probe to bcm_get_resources
From: Hans de Goede @ 2017-10-02 15:23 UTC (permalink / raw)
  To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg
  Cc: Hans de Goede, Frédéric Danis, Sebastian Reichel, robh,
	linux-bluetooth, linux-serial, linux-acpi
In-Reply-To: <20171002152356.4714-1-hdegoede@redhat.com>

After our previous changes, there is nothing platform specific about
bcm_platform_probe anymore, rename it to bcm_get_resources.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/bluetooth/hci_bcm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index cfc87fb5051c..5c8371d8aace 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -756,7 +756,7 @@ static int bcm_resource(struct acpi_resource *ares, void *data)
 }
 #endif /* CONFIG_ACPI */
 
-static int bcm_platform_probe(struct bcm_device *dev)
+static int bcm_get_resources(struct bcm_device *dev)
 {
 	dev->name = dev_name(dev->dev);
 
@@ -857,7 +857,7 @@ static int bcm_probe(struct platform_device *pdev)
 			return ret;
 	}
 
-	ret = bcm_platform_probe(dev);
+	ret = bcm_get_resources(dev);
 	if (ret)
 		return ret;
 
-- 
2.14.2


^ permalink raw reply related

* [PATCH 7/9] Bluetooth: hci_bcm: Make acpi_probe get irq from ACPI resources
From: Hans de Goede @ 2017-10-02 15:23 UTC (permalink / raw)
  To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg
  Cc: Hans de Goede, Frédéric Danis, Sebastian Reichel, robh,
	linux-bluetooth, linux-serial, linux-acpi
In-Reply-To: <20171002152356.4714-1-hdegoede@redhat.com>

The ACPI subsys is going to move over to instantiating ACPI enumerated
HCIs as serdevs, rather then as platform devices.

So we need to make bcm_acpi_probe() suitable for use on non platform-
devices too, which means that we cannot rely on platform_get_irq()
getting called.

This commit modifies bcm_acpi_probe() to directly get the irq from
the ACPI resources, this is a preparation patch for adding (runtime)pm
support to the serdev path.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/bluetooth/hci_bcm.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 5c8371d8aace..48a428909958 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -805,6 +805,7 @@ static int bcm_acpi_probe(struct bcm_device *dev)
 	const struct dmi_system_id *dmi_id;
 	const struct acpi_gpio_mapping *gpio_mapping = acpi_bcm_int_last_gpios;
 	const struct acpi_device_id *id;
+	struct resource_entry *entry;
 	int ret;
 
 	/* Retrieve GPIO data */
@@ -821,6 +822,13 @@ static int bcm_acpi_probe(struct bcm_device *dev)
 				     &resources, bcm_resource, dev);
 	if (ret < 0)
 		return ret;
+
+	resource_list_for_each_entry(entry, &resources) {
+		if (resource_type(entry->res) == IORESOURCE_IRQ) {
+			dev->irq = entry->res->start;
+			break;
+		}
+	}
 	acpi_dev_free_resource_list(&resources);
 
 	dmi_id = dmi_first_match(bcm_active_low_irq_dmi_table);
-- 
2.14.2


^ permalink raw reply related

* [PATCH 8/9] Bluetooth: hci_bcm: Make suspend/resume functions platform_dev independent
From: Hans de Goede @ 2017-10-02 15:23 UTC (permalink / raw)
  To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg
  Cc: Hans de Goede, Frédéric Danis, Sebastian Reichel,
	robh-DgEjT+Ai2ygdnm+yROfE0A,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171002152356.4714-1-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Use dev_get_drvdata instead of platform_get_drvdata in the suspend /
resume functions. This is a preparation patch for adding (runtime)pm
support to the serdev path.

Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 drivers/bluetooth/hci_bcm.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 48a428909958..7191cee5ced0 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -558,7 +558,7 @@ static struct sk_buff *bcm_dequeue(struct hci_uart *hu)
 #ifdef CONFIG_PM
 static int bcm_suspend_device(struct device *dev)
 {
-	struct bcm_device *bdev = platform_get_drvdata(to_platform_device(dev));
+	struct bcm_device *bdev = dev_get_drvdata(dev);
 
 	bt_dev_dbg(bdev, "");
 
@@ -581,7 +581,7 @@ static int bcm_suspend_device(struct device *dev)
 
 static int bcm_resume_device(struct device *dev)
 {
-	struct bcm_device *bdev = platform_get_drvdata(to_platform_device(dev));
+	struct bcm_device *bdev = dev_get_drvdata(dev);
 
 	bt_dev_dbg(bdev, "");
 
@@ -606,7 +606,7 @@ static int bcm_resume_device(struct device *dev)
 /* Platform suspend callback */
 static int bcm_suspend(struct device *dev)
 {
-	struct bcm_device *bdev = platform_get_drvdata(to_platform_device(dev));
+	struct bcm_device *bdev = dev_get_drvdata(dev);
 	int error;
 
 	bt_dev_dbg(bdev, "suspend: is_suspended %d", bdev->is_suspended);
@@ -638,7 +638,7 @@ static int bcm_suspend(struct device *dev)
 /* Platform resume callback */
 static int bcm_resume(struct device *dev)
 {
-	struct bcm_device *bdev = platform_get_drvdata(to_platform_device(dev));
+	struct bcm_device *bdev = dev_get_drvdata(dev);
 
 	bt_dev_dbg(bdev, "resume: is_suspended %d", bdev->is_suspended);
 
-- 
2.14.2

^ permalink raw reply related

* [PATCH 9/9] Bluetooth: hci_bcm: Add (runtime)pm support to the serdev driver
From: Hans de Goede @ 2017-10-02 15:23 UTC (permalink / raw)
  To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg
  Cc: Hans de Goede, Frédéric Danis, Sebastian Reichel,
	robh-DgEjT+Ai2ygdnm+yROfE0A,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171002152356.4714-1-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Make the serdev driver use struct bcm_device as its driver data and share
all the pm / GPIO / IRQ related code paths with the platform driver.

After this commit the 2 drivers are in essence the same and the serdev
driver interface can be used for all ACPI enumerated HCI UARTs.

Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 drivers/bluetooth/hci_bcm.c | 118 +++++++++++++++++++++++++-------------------
 1 file changed, 68 insertions(+), 50 deletions(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 7191cee5ced0..5fff99bb6444 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -52,8 +52,10 @@
 
 #define BCM_AUTOSUSPEND_DELAY	5000 /* default autosleep delay */
 
-/* platform device driver resources */
+/* device driver resources */
 struct bcm_device {
+	/* Must be the first member, hci_serdev.c expects this. */
+	struct hci_uart		serdev_hu;
 	struct list_head	list;
 
 	struct device		*dev;
@@ -76,11 +78,6 @@ struct bcm_device {
 #endif
 };
 
-/* serdev driver resources */
-struct bcm_serdev {
-	struct hci_uart hu;
-};
-
 /* generic bcm uart resources */
 struct bcm_data {
 	struct sk_buff		*rx_skb;
@@ -155,6 +152,10 @@ static bool bcm_device_exists(struct bcm_device *device)
 {
 	struct list_head *p;
 
+	/* Devices using serdev always exist */
+	if (device && device->hu && device->hu->serdev)
+		return true;
+
 	list_for_each(p, &bcm_device_list) {
 		struct bcm_device *dev = list_entry(p, struct bcm_device, list);
 
@@ -200,7 +201,6 @@ static int bcm_request_irq(struct bcm_data *bcm)
 	struct bcm_device *bdev = bcm->dev;
 	int err;
 
-	/* If this is not a platform device, do not enable PM functionalities */
 	mutex_lock(&bcm_device_lock);
 	if (!bcm_device_exists(bdev)) {
 		err = -ENODEV;
@@ -313,18 +313,17 @@ static int bcm_open(struct hci_uart *hu)
 
 	hu->priv = bcm;
 
-	/* If this is a serdev defined device, then only use
-	 * serdev open primitive and skip the rest.
-	 */
+	mutex_lock(&bcm_device_lock);
+
 	if (hu->serdev) {
 		serdev_device_open(hu->serdev);
+		bcm->dev = serdev_device_get_drvdata(hu->serdev);
 		goto out;
 	}
 
 	if (!hu->tty->dev)
 		goto out;
 
-	mutex_lock(&bcm_device_lock);
 	list_for_each(p, &bcm_device_list) {
 		struct bcm_device *dev = list_entry(p, struct bcm_device, list);
 
@@ -334,37 +333,45 @@ static int bcm_open(struct hci_uart *hu)
 		 */
 		if (hu->tty->dev->parent == dev->dev->parent) {
 			bcm->dev = dev;
-			hu->init_speed = dev->init_speed;
-			hu->oper_speed = dev->oper_speed;
 #ifdef CONFIG_PM
 			dev->hu = hu;
 #endif
-			bcm_gpio_set_power(bcm->dev, true);
 			break;
 		}
 	}
 
-	mutex_unlock(&bcm_device_lock);
 out:
+	if (bcm->dev) {
+		hu->init_speed = bcm->dev->init_speed;
+		hu->oper_speed = bcm->dev->oper_speed;
+		bcm_gpio_set_power(bcm->dev, true);
+	}
+
+	mutex_unlock(&bcm_device_lock);
 	return 0;
 }
 
 static int bcm_close(struct hci_uart *hu)
 {
 	struct bcm_data *bcm = hu->priv;
-	struct bcm_device *bdev = bcm->dev;
+	struct bcm_device *bdev = NULL;
 
 	bt_dev_dbg(hu->hdev, "hu %p", hu);
 
-	/* If this is a serdev defined device, only use serdev
-	 * close primitive and then continue as usual.
-	 */
-	if (hu->serdev)
-		serdev_device_close(hu->serdev);
-
 	/* Protect bcm->dev against removal of the device or driver */
 	mutex_lock(&bcm_device_lock);
-	if (bcm_device_exists(bdev)) {
+
+	if (hu->serdev) {
+		serdev_device_close(hu->serdev);
+		bdev = serdev_device_get_drvdata(hu->serdev);
+	} else if (bcm_device_exists(bcm->dev)) {
+		bdev = bcm->dev;
+#ifdef CONFIG_PM
+		bdev->hu = NULL;
+#endif
+	}
+
+	if (bdev) {
 		bcm_gpio_set_power(bdev, false);
 #ifdef CONFIG_PM
 		pm_runtime_disable(bdev->dev);
@@ -374,8 +381,6 @@ static int bcm_close(struct hci_uart *hu)
 			devm_free_irq(bdev->dev, bdev->irq, bdev);
 			device_init_wakeup(bdev->dev, false);
 		}
-
-		bdev->hu = NULL;
 #endif
 	}
 	mutex_unlock(&bcm_device_lock);
@@ -603,7 +608,7 @@ static int bcm_resume_device(struct device *dev)
 #endif
 
 #ifdef CONFIG_PM_SLEEP
-/* Platform suspend callback */
+/* suspend callback */
 static int bcm_suspend(struct device *dev)
 {
 	struct bcm_device *bdev = dev_get_drvdata(dev);
@@ -611,8 +616,10 @@ static int bcm_suspend(struct device *dev)
 
 	bt_dev_dbg(bdev, "suspend: is_suspended %d", bdev->is_suspended);
 
-	/* bcm_suspend can be called at any time as long as platform device is
-	 * bound, so it should use bcm_device_lock to protect access to hci_uart
+	/*
+	 * When used with a device instantiated as platform_device, bcm_suspend
+	 * can be called at any time as long as the platform device is bound,
+	 * so it should use bcm_device_lock to protect access to hci_uart
 	 * and device_wake-up GPIO.
 	 */
 	mutex_lock(&bcm_device_lock);
@@ -635,15 +642,17 @@ static int bcm_suspend(struct device *dev)
 	return 0;
 }
 
-/* Platform resume callback */
+/* resume callback */
 static int bcm_resume(struct device *dev)
 {
 	struct bcm_device *bdev = dev_get_drvdata(dev);
 
 	bt_dev_dbg(bdev, "resume: is_suspended %d", bdev->is_suspended);
 
-	/* bcm_resume can be called at any time as long as platform device is
-	 * bound, so it should use bcm_device_lock to protect access to hci_uart
+	/*
+	 * When used with a device instantiated as platform_device, bcm_resume
+	 * can be called at any time as long as platform device is bound,
+	 * so it should use bcm_device_lock to protect access to hci_uart
 	 * and device_wake-up GPIO.
 	 */
 	mutex_lock(&bcm_device_lock);
@@ -786,15 +795,6 @@ static int bcm_get_resources(struct bcm_device *dev)
 	}
 
 	dev_info(dev->dev, "BCM irq: %d\n", dev->irq);
-
-	/* Make sure at-least one of the GPIO is defined and that
-	 * a name is specified for this instance
-	 */
-	if ((!dev->device_wakeup && !dev->shutdown) || !dev->name) {
-		dev_err(dev->dev, "invalid platform data\n");
-		return -EINVAL;
-	}
-
 	return 0;
 }
 
@@ -847,6 +847,12 @@ static int bcm_acpi_probe(struct bcm_device *dev)
 }
 #endif /* CONFIG_ACPI */
 
+static int bcm_of_probe(struct bcm_device *bdev)
+{
+	device_property_read_u32(bdev->dev, "max-speed", &bdev->oper_speed);
+	return 0;
+}
+
 static int bcm_probe(struct platform_device *pdev)
 {
 	struct bcm_device *dev;
@@ -935,7 +941,7 @@ static const struct acpi_device_id bcm_acpi_match[] = {
 MODULE_DEVICE_TABLE(acpi, bcm_acpi_match);
 #endif
 
-/* Platform suspend and resume callbacks */
+/* suspend and resume callbacks */
 static const struct dev_pm_ops bcm_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(bcm_suspend, bcm_resume)
 	SET_RUNTIME_PM_OPS(bcm_suspend_device, bcm_resume_device, NULL)
@@ -953,29 +959,39 @@ static struct platform_driver bcm_driver = {
 
 static int bcm_serdev_probe(struct serdev_device *serdev)
 {
-	struct bcm_serdev *bcmdev;
-	u32 speed;
+	struct bcm_device *bcmdev;
 	int err;
 
 	bcmdev = devm_kzalloc(&serdev->dev, sizeof(*bcmdev), GFP_KERNEL);
 	if (!bcmdev)
 		return -ENOMEM;
 
-	bcmdev->hu.serdev = serdev;
+	bcmdev->dev = &serdev->dev;
+	bcmdev->hu = &bcmdev->serdev_hu;
+	bcmdev->serdev_hu.serdev = serdev;
 	serdev_device_set_drvdata(serdev, bcmdev);
 
-	err = device_property_read_u32(&serdev->dev, "max-speed", &speed);
-	if (!err)
-		bcmdev->hu.oper_speed = speed;
+	if (has_acpi_companion(&serdev->dev))
+		err = bcm_acpi_probe(bcmdev);
+	else
+		err = bcm_of_probe(bcmdev);
+	if (err)
+		return err;
 
-	return hci_uart_register_device(&bcmdev->hu, &bcm_proto);
+	err = bcm_get_resources(bcmdev);
+	if (err)
+		return err;
+
+	bcm_gpio_set_power(bcmdev, false);
+
+	return hci_uart_register_device(&bcmdev->serdev_hu, &bcm_proto);
 }
 
 static void bcm_serdev_remove(struct serdev_device *serdev)
 {
-	struct bcm_serdev *bcmdev = serdev_device_get_drvdata(serdev);
+	struct bcm_device *bcmdev = serdev_device_get_drvdata(serdev);
 
-	hci_uart_unregister_device(&bcmdev->hu);
+	hci_uart_unregister_device(&bcmdev->serdev_hu);
 }
 
 #ifdef CONFIG_OF
@@ -992,6 +1008,8 @@ static struct serdev_device_driver bcm_serdev_driver = {
 	.driver = {
 		.name = "hci_uart_bcm",
 		.of_match_table = of_match_ptr(bcm_bluetooth_of_match),
+		.acpi_match_table = ACPI_PTR(bcm_acpi_match),
+		.pm = &bcm_pm_ops,
 	},
 };
 
-- 
2.14.2

^ permalink raw reply related

* Re: [RFC,3/3] Bluetooth: hci_bcm: Add ACPI serdev support for BCM2E39
From: Hans de Goede @ 2017-10-02 15:26 UTC (permalink / raw)
  To: Frédéric Danis, robh, marcel, sre, loic.poulain
  Cc: linux-bluetooth, linux-serial, linux-acpi
In-Reply-To: <be4075c9-110a-1415-3d9b-a631fe91b34f@redhat.com>

Hi,

On 02-10-17 09:07, Hans de Goede wrote:
> Hi,
> 
> First of all Frédéric, thank you very much for working on this,
> this is a very much needed patch series.
> 
> I've been testing this on a GPD pocket which has a BCM2E7E
> ACPI device for the BT which is part of the BCM43562A wifi/bt
> combo on this device.
> 
> 2 remarks:
> 
> 1) The following errors show up with this series:
> 
> [   87.059091] synth uevent: /devices/pci0000:00/8086228A:00/serial1: failed to send uevent
> [   87.059097] serial serial1: uevent: failed to send synthetic uevent
> [   87.059178] synth uevent: /devices/pci0000:00/8086228A:01/serial2: failed to send uevent
> [   87.059180] serial serial2: uevent: failed to send synthetic uevent
> [   87.062665] synth uevent: /devices/pnp0/00:01/serial0: failed to send uevent
> [   87.062671] serial serial0: uevent: failed to send synthetic uevent
> 
> This needs to be fixed :)  I haven't looked into this yet.
> 
> 2) As I already suspected when reading the patches, we cannot
> move the ACPI ids over 1 by 1. The first 2 patches in the series
> cause all BCM2E* ACPI devices to no longer get enumerated as
> platform devices, instead they now get enumerated as serdevs.
> 
> So adding only the known to work ACPI ids to the acpi_match_table
> for the bcm_serdev_driver has the undesirable result that it makes
> sure that the other ACPI-ids will not work, as there are not
> platform devices instantiated for the platform driver to bind to.
> 
> I started with simplifying your patch to this:
> 
>  From f8728f5440c85f7e5584ac1eafa1b090b2042276 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Danis?= <frederic.danis.oss@gmail.com>
> Date: Thu, 7 Sep 2017 14:10:14 +0200
> Subject: [PATCH] Bluetooth: hci_bcm: Make ACPI enumerated HCIs use serdev
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> Now that the ACPI subsys instantiates ACPI enumerated HCIs as serdevs,
> the ACPI ids for them should be part of the bcm_serdev_driver's
> acpi_match_table.
> 
> Signed-off-by: Frédéric Danis <frederic.danis.oss@gmail.com>
> [hdegoede: Move all ACPI ids over]
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>    drivers/bluetooth/hci_bcm.c | 2 +-
>    1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
> index 2285ca673ae3..4db777bb0049 100644
> --- a/drivers/bluetooth/hci_bcm.c
> +++ b/drivers/bluetooth/hci_bcm.c
> @@ -942,7 +942,6 @@ static struct platform_driver bcm_driver = {
>        .remove = bcm_remove,
>        .driver = {
>            .name = "hci_bcm",
> -        .acpi_match_table = ACPI_PTR(bcm_acpi_match),
>            .pm = &bcm_pm_ops,
>        },
>    };
> @@ -988,6 +987,7 @@ static struct serdev_device_driver bcm_serdev_driver = {
>        .driver = {
>            .name = "hci_uart_bcm",
>            .of_match_table = of_match_ptr(bcm_bluetooth_of_match),
> +        .acpi_match_table = ACPI_PTR(bcm_acpi_match),
>        },
>    };
> 
> -- 
> 2.14.2
> 
> Unfortunately that is not good enough. The platform driver code also
> has (runtime) pm support using gpios and a host-wake interrupt, which
> the serdev code all lacks and without driving the shutdown gpio to not
> shutdown the BT device connected to the UART we not only are lacking
> pm support, but BT does not work at all.
> 
> So yesterday evening I did a patch to merge all that into the serdev hci_bcm
> code and throw away the platform code as I don't think any users will be left
> after this. With this functionality for the BCM2E7E ACPI device is restored
> again, without needing to do a btattach from userspace, which is great :)
> 
> I've attached the resulting patch. In the mean time I've realized that
> this approach is going to make merging hard. So I'm going to redo the changes
> so that we can have both a complete (with runtime-pm, gpios, etc.) serdev
> driver and keep the platform driver for now. Then we can first merge the
> hci_bcm changes to make the serdev driver side complete and once those are
> merged merge the matching ACPI subsys changes without having a window in
> between where things will not work.

Ok, I've just finished and send out v1 of a patch-series adding (runtime)pm,
GPIO and IRQ support to the serdev based code paths in hci_bcm.c .

Regards,

Hans

^ permalink raw reply

* Re: [PATCH 1/9] Bluetooth: hci_uart_set_flow_control: Fix NULL deref when using serdev
From: Sebastian Reichel @ 2017-10-02 22:34 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Marcel Holtmann, Gustavo Padovan, Johan Hedberg,
	Frédéric Danis, robh, linux-bluetooth, linux-serial,
	linux-acpi
In-Reply-To: <20171002152356.4714-2-hdegoede@redhat.com>

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

Hi,

On Mon, Oct 02, 2017 at 05:23:48PM +0200, Hans de Goede wrote:
> Fix a NULL pointer deref (hu->tty) when calling hci_uart_set_flow_control
> on hci_uart-s using serdev.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/bluetooth/hci_ldisc.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
> index a746627e784e..d02f6d029093 100644
> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -41,6 +41,7 @@
>  #include <linux/ioctl.h>
>  #include <linux/skbuff.h>
>  #include <linux/firmware.h>
> +#include <linux/serdev.h>
>  
>  #include <net/bluetooth/bluetooth.h>
>  #include <net/bluetooth/hci_core.h>
> @@ -298,6 +299,11 @@ void hci_uart_set_flow_control(struct hci_uart *hu, bool enable)
>  	unsigned int set = 0;
>  	unsigned int clear = 0;
>  
> +	if (hu->serdev) {
> +		serdev_device_set_flow_control(hu->serdev, !enable);
> +		return;
> +	}

I think this should also control rts, so that the behaviour is the
same for serdev and ldisc implementation:

serdev_device_set_rts(serdev, enable);

> +
>  	if (enable) {
>  		/* Disable hardware flow control */
>  		ktermios = tty->termios;

-- Sebastian

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

^ permalink raw reply

* [PATCH V1 0/3] High baud rate supports of F81866/F81216H
From: Ji-Ze Hong (Peter Hong) @ 2017-10-03  3:08 UTC (permalink / raw)
  To: gregkh, jslaby
  Cc: rel+kernel, linux-serial, linux-kernel, peter_hong,
	Ji-Ze Hong (Peter Hong)

The Fintek F81866/F81216H support high baud rate and it's up to 1.5Mbps
with 24MHz clock source. It's also support 500Kbps via 24MHz clock too.

We'll implements clock source checking in function fintek_8250_set_termios().

Ji-Ze Hong (Peter Hong) (3):
  serial: 8250_fintek: UART dynamic clocksource on Fintek F81866
  serial: 8250_fintek: UART dynamic clocksource on Fintek F81216H
  serial: 8250_fintek: fix warning reported from smatch

 drivers/tty/serial/8250/8250_fintek.c | 84 ++++++++++++++++++++++++++++++++++-
 1 file changed, 83 insertions(+), 1 deletion(-)

-- 
1.9.1

^ permalink raw reply

* [PATCH V1 1/3] serial: 8250_fintek: UART dynamic clocksource on Fintek F81866
From: Ji-Ze Hong (Peter Hong) @ 2017-10-03  3:08 UTC (permalink / raw)
  To: gregkh, jslaby
  Cc: rel+kernel, linux-serial, linux-kernel, peter_hong,
	Ji-Ze Hong (Peter Hong)
In-Reply-To: <1507000116-13682-1-git-send-email-hpeter+linux_kernel@gmail.com>

The F81866 had 4 clocksource 1.8432/18.432/14.769/24MHz and baud rates can
be up to 1.5Mbits with 24MHz. We'll implements the dynamic clocksource in
fintek_8250_set_termios().

Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
---
 drivers/tty/serial/8250/8250_fintek.c | 54 +++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_fintek.c b/drivers/tty/serial/8250/8250_fintek.c
index e500f7d..53ea353 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -287,6 +287,59 @@ static void fintek_8250_goto_highspeed(struct uart_8250_port *uart,
 	}
 }
 
+void fintek_8250_set_termios(struct uart_port *port, struct ktermios *termios,
+			struct ktermios *old)
+{
+	struct fintek_8250 *pdata = port->private_data;
+	unsigned int baud = tty_termios_baud_rate(termios);
+	int i;
+	static u32 baudrate_table[] = {115200, 921600, 1152000, 1500000};
+	static u8 clock_table[] = { F81866_UART_CLK_1_8432MHZ,
+			F81866_UART_CLK_14_769MHZ, F81866_UART_CLK_18_432MHZ,
+			F81866_UART_CLK_24MHZ };
+
+	for (i = 0; i < ARRAY_SIZE(baudrate_table); ++i) {
+		if (baud > baudrate_table[i] || baudrate_table[i] % baud != 0)
+			continue;
+
+		if (port->uartclk == baudrate_table[i] * 16)
+			break;
+
+		if (fintek_8250_enter_key(pdata->base_port, pdata->key))
+			continue;
+
+		port->uartclk = baudrate_table[i] * 16;
+
+		sio_write_reg(pdata, LDN, pdata->index);
+		sio_write_mask_reg(pdata, F81866_UART_CLK,
+				F81866_UART_CLK_MASK, clock_table[i]);
+
+		fintek_8250_exit_key(pdata->base_port);
+		break;
+	}
+
+	if (i == ARRAY_SIZE(baudrate_table)) {
+		baud = tty_termios_baud_rate(old);
+		tty_termios_encode_baud_rate(termios, baud, baud);
+	}
+
+	serial8250_do_set_termios(port, termios, old);
+}
+
+static void fintek_8250_set_termios_handler(struct uart_8250_port *uart)
+{
+	struct fintek_8250 *pdata = uart->port.private_data;
+
+	switch (pdata->pid) {
+	case CHIP_ID_F81866:
+		uart->port.set_termios = fintek_8250_set_termios;
+		break;
+
+	default:
+		break;
+	}
+}
+
 static int probe_setup_port(struct fintek_8250 *pdata,
 					struct uart_8250_port *uart)
 {
@@ -373,6 +426,7 @@ int fintek_8250_probe(struct uart_8250_port *uart)
 	memcpy(pdata, &probe_data, sizeof(probe_data));
 	uart->port.private_data = pdata;
 	fintek_8250_set_rs485_handler(uart);
+	fintek_8250_set_termios_handler(uart);
 
 	return 0;
 }
-- 
1.9.1

^ permalink raw reply related

* [PATCH V1 2/3] serial: 8250_fintek: UART dynamic clocksource on Fintek F81216H
From: Ji-Ze Hong (Peter Hong) @ 2017-10-03  3:08 UTC (permalink / raw)
  To: gregkh, jslaby
  Cc: rel+kernel, linux-serial, linux-kernel, peter_hong,
	Ji-Ze Hong (Peter Hong)
In-Reply-To: <1507000116-13682-1-git-send-email-hpeter+linux_kernel@gmail.com>

The F81216H had 4 clocksource 1.8432/18.432/14.769/24MHzand  baud rates can
be up to 1.5Mbits with 24MHz. The register value and mask is the same with
F81866. But F81866 register address is F2h, F81216H is F0h.

We'll implements the dynamic clocksource in fintek_8250_set_termios().

Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
---
 drivers/tty/serial/8250/8250_fintek.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_fintek.c b/drivers/tty/serial/8250/8250_fintek.c
index 53ea353..f3b622f 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -40,6 +40,16 @@
 #define IRQ_LEVEL_LOW	0
 #define IRQ_EDGE_HIGH	BIT(5)
 
+/*
+ * F81216H clock source register, the value and mask is the same with F81866,
+ * but it's on F0h.
+ *
+ * Clock speeds for UART (register F0h)
+ * 00: 1.8432MHz.
+ * 01: 18.432MHz.
+ * 10: 24MHz.
+ * 11: 14.769MHz.
+ */
 #define RS485  0xF0
 #define RTS_INVERT BIT(5)
 #define RS485_URA BIT(4)
@@ -293,11 +303,28 @@ void fintek_8250_set_termios(struct uart_port *port, struct ktermios *termios,
 	struct fintek_8250 *pdata = port->private_data;
 	unsigned int baud = tty_termios_baud_rate(termios);
 	int i;
+	u8 reg;
 	static u32 baudrate_table[] = {115200, 921600, 1152000, 1500000};
 	static u8 clock_table[] = { F81866_UART_CLK_1_8432MHZ,
 			F81866_UART_CLK_14_769MHZ, F81866_UART_CLK_18_432MHZ,
 			F81866_UART_CLK_24MHZ };
 
+	switch (pdata->pid) {
+	case CHIP_ID_F81216H:
+		reg = RS485;
+		break;
+	case CHIP_ID_F81866:
+		reg = F81866_UART_CLK;
+		break;
+	default:
+		/* Don't change clocksource with unknown PID */
+		dev_warn(port->dev,
+			"%s: pid: %x Not support. use default set_termios.\n",
+			__func__, pdata->pid);
+		serial8250_do_set_termios(port, termios, old);
+		return;
+	}
+
 	for (i = 0; i < ARRAY_SIZE(baudrate_table); ++i) {
 		if (baud > baudrate_table[i] || baudrate_table[i] % baud != 0)
 			continue;
@@ -311,8 +338,8 @@ void fintek_8250_set_termios(struct uart_port *port, struct ktermios *termios,
 		port->uartclk = baudrate_table[i] * 16;
 
 		sio_write_reg(pdata, LDN, pdata->index);
-		sio_write_mask_reg(pdata, F81866_UART_CLK,
-				F81866_UART_CLK_MASK, clock_table[i]);
+		sio_write_mask_reg(pdata, reg, F81866_UART_CLK_MASK,
+				clock_table[i]);
 
 		fintek_8250_exit_key(pdata->base_port);
 		break;
@@ -331,6 +358,7 @@ static void fintek_8250_set_termios_handler(struct uart_8250_port *uart)
 	struct fintek_8250 *pdata = uart->port.private_data;
 
 	switch (pdata->pid) {
+	case CHIP_ID_F81216H:
 	case CHIP_ID_F81866:
 		uart->port.set_termios = fintek_8250_set_termios;
 		break;
-- 
1.9.1

^ permalink raw reply related

* [PATCH V1 3/3] serial: 8250_fintek: fix warning reported from smatch
From: Ji-Ze Hong (Peter Hong) @ 2017-10-03  3:08 UTC (permalink / raw)
  To: gregkh, jslaby
  Cc: rel+kernel, linux-serial, linux-kernel, peter_hong,
	Ji-Ze Hong (Peter Hong)
In-Reply-To: <1507000116-13682-1-git-send-email-hpeter+linux_kernel@gmail.com>

This patch is fix the warning reported by smatch as following:

drivers/tty/serial/8250/8250_fintek.c:294 fintek_8250_goto_highspeed()
warn: inconsistent indenting

Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
---
 drivers/tty/serial/8250/8250_fintek.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_fintek.c b/drivers/tty/serial/8250/8250_fintek.c
index f3b622f..96cc45f 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -290,7 +290,7 @@ static void fintek_8250_goto_highspeed(struct uart_8250_port *uart,
 			F81866_UART_CLK_MASK,
 			F81866_UART_CLK_14_769MHZ);
 
-			uart->port.uartclk = 921600 * 16;
+		uart->port.uartclk = 921600 * 16;
 		break;
 	default: /* leave clock speed untouched */
 		break;
-- 
1.9.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox