Linux Serial subsystem development
 help / color / mirror / Atom feed
* Re: [PATCH] serial: imx-serial - move DMA buffer configuration to DT
From: Uwe Kleine-König @ 2017-10-04 21:49 UTC (permalink / raw)
  To: Han, Nandor (GE Healthcare)
  Cc: Romain Perier, Greg Kroah-Hartman, Jiri Slaby,
	linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <AM3P101MB0180F7019908E059648488F0E67D0-Irc2Ng3OI610aGNDtyle9VAr667LJVwUiGd9ebBGJoev3QGu/rdwKA@public.gmane.org>

Hello,

On Mon, Oct 02, 2017 at 01:17:41PM +0000, Han, Nandor (GE Healthcare) wrote:
> > -----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.

My question didn't target where to configure the buffer size instead of
dts. I wonder if it would help to change the fifo watermark limits for
example.

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

BTW In the past I saw the serial core introduce latency, too. Are you
sure that's not your bottle neck?

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

The problem is: How should a person, who wants to make available a port
on a machine via dts, choose what value to use for fsl,dma-size?

What you want (for a low latency port) is that also small amounts of
data received are passed quickly to the upper layer. The knob you
identified to be available for that is the dma buffer size.

I'd prefer to talk about low latency instead of buffer sizes when
setting parameters for the port. That this influences the buffer size
(and maybe watermark settings) under the hood shouldn't matter for the
person configuring the low latency property.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
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 v2 9/9] Bluetooth: hci_bcm: Add (runtime)pm support to the serdev driver
From: Hans de Goede @ 2017-10-04 18:43 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: <20171004184343.7855-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 e7494efb56e9..cff91930f0b1 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);
@@ -785,15 +794,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;
 }
 
@@ -846,6 +846,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;
@@ -934,7 +940,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)
@@ -952,29 +958,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
@@ -991,6 +1007,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

* [PATCH v2 8/9] Bluetooth: hci_bcm: Make suspend/resume functions platform_dev independent
From: Hans de Goede @ 2017-10-04 18:43 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: <20171004184343.7855-1-hdegoede@redhat.com>

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@redhat.com>
---
 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 e577547c4acf..e7494efb56e9 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 v2 7/9] Bluetooth: hci_bcm: Make acpi_probe get irq from ACPI resources
From: Hans de Goede @ 2017-10-04 18:43 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: <20171004184343.7855-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>
---
Changes in v2:
-Make bcm_resource return 0 so that acpi_dev_get_resources() does not
 return an empty list (Suggested-by: Frédéric Danis)
---
 drivers/bluetooth/hci_bcm.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 5c8371d8aace..e577547c4acf 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -751,8 +751,7 @@ static int bcm_resource(struct acpi_resource *ares, void *data)
 		break;
 	}
 
-	/* Always tell the ACPI core to skip this resource */
-	return 1;
+	return 0;
 }
 #endif /* CONFIG_ACPI */
 
@@ -805,6 +804,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 +821,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 v2 6/9] Bluetooth: hci_bcm: Rename bcm_platform_probe to bcm_get_resources
From: Hans de Goede @ 2017-10-04 18:43 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: <20171004184343.7855-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 v2 5/9] Bluetooth: hci_bcm: Store device pointer instead of platform_device pointer
From: Hans de Goede @ 2017-10-04 18:43 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: <20171004184343.7855-1-hdegoede@redhat.com>

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@redhat.com>
---
 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 v2 4/9] Bluetooth: hci_bcm: Move platform_get_irq call to bcm_probe
From: Hans de Goede @ 2017-10-04 18:43 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: <20171004184343.7855-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 v2 3/9] Bluetooth: hci_bcm: Move bcm_platform_probe call out of bcm_acpi_probe
From: Hans de Goede @ 2017-10-04 18:43 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: <20171004184343.7855-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 v2 2/9] Bluetooth: hci_bcm: Fix setting of irq trigger type
From: Hans de Goede @ 2017-10-04 18:43 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: <20171004184343.7855-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 v2 1/9] Bluetooth: hci_uart_set_flow_control: Fix NULL deref when using serdev
From: Hans de Goede @ 2017-10-04 18:43 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

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>
---
Changes in v2:
-Also set RTS (Suggested-by: Sebastian Reichel <sre@kernel.org>)
---
 drivers/bluetooth/hci_ldisc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index a746627e784e..eec95019f15c 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,12 @@ 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);
+		serdev_device_set_rts(hu->serdev, !enable);
+		return;
+	}
+
 	if (enable) {
 		/* Disable hardware flow control */
 		ktermios = tty->termios;
-- 
2.14.2


^ permalink raw reply related

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

Hi,

On 03-10-17 00:34, Sebastian Reichel wrote:
> 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);

Good point, fixed for v2 of this set (which I will send out right
after this mail).

Regards,

Hans

^ permalink raw reply

* Re: [PATCH v2] serial: imx: Correct comment imx_flush_buffer()
From: Uwe Kleine-König @ 2017-10-04 17:32 UTC (permalink / raw)
  To: Martyn Welch; +Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel
In-Reply-To: <1507133607-22966-1-git-send-email-martyn.welch@collabora.co.uk>

On Wed, Oct 04, 2017 at 05:13:27PM +0100, Martyn Welch wrote:
> The comment in imx_flush_buffer() states that the state of 4 registers
> are to be saved/restored, then only saves and restores 3 registers. The
> missing register (UBRC) is read only and thus can't be restored.
> 
> Update the comment to reflect reality.
> 
> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* [PATCH v2] serial: imx: Correct comment imx_flush_buffer()
From: Martyn Welch @ 2017-10-04 16:13 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-serial, linux-kernel, Martyn Welch

The comment in imx_flush_buffer() states that the state of 4 registers
are to be saved/restored, then only saves and restores 3 registers. The
missing register (UBRC) is read only and thus can't be restored.

Update the comment to reflect reality.

Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
---

v2: Remove trailing whitespace.

 drivers/tty/serial/imx.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index b697c1e..76818a2 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1417,10 +1417,14 @@ static void imx_flush_buffer(struct uart_port *port)
 
 	/*
 	 * According to the Reference Manual description of the UART SRST bit:
+	 *
 	 * "Reset the transmit and receive state machines,
 	 * all FIFOs and register USR1, USR2, UBIR, UBMR, UBRC, URXD, UTXD
-	 * and UTS[6-3]". As we don't need to restore the old values from
-	 * USR1, USR2, URXD, UTXD, only save/restore the other four registers
+	 * and UTS[6-3]".
+	 *
+	 * We don't need to restore the old values from USR1, USR2, URXD and
+	 * UTXD. UBRC is read only, so only save/restore the other three
+	 * registers.
 	 */
 	ubir = readl(sport->port.membase + UBIR);
 	ubmr = readl(sport->port.membase + UBMR);
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH] serial: imx: Correct comment imx_flush_buffer()
From: Martyn Welch @ 2017-10-04 16:12 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-serial, linux-kernel
In-Reply-To: <20171003182755.GA32455@kroah.com>

On Tue, 2017-10-03 at 20:27 +0200, Greg Kroah-Hartman wrote:
> On Fri, Sep 29, 2017 at 10:22:19AM +0100, Martyn Welch wrote:
> > The comment in imx_flush_buffer() states that the state of 4 registers
> > are to be saved/restored, then only saves and restores 3 registers. The
> > missing register (UBRC) is read only and thus can't be restored.
> > 
> > Update the comment to reflect reality.
> 
> Always run checkpatch.pl so you don't get grumpy maintainers telling you
> to run checkpatch.pl :(

Sorry Greg, must have got out of sync on that one :-(

Martyn

^ permalink raw reply

* Re: [PATCH 7/9] Bluetooth: hci_bcm: Make acpi_probe get irq from ACPI resources
From: Hans de Goede @ 2017-10-04 13:26 UTC (permalink / raw)
  To: Frédéric Danis, Marcel Holtmann, Gustavo Padovan,
	Johan Hedberg
  Cc: Sebastian Reichel, robh, linux-bluetooth, linux-serial,
	linux-acpi
In-Reply-To: <c06ae2d7-9628-ad70-2509-eca18ba16f0a@gmail.com>

Hi,

On 03-10-17 19:21, Frédéric Danis wrote:
> Hi Hans,
> 
> First of all, many thanks for your work on this, it helped me a lot to get full Bluetooth on my Asus T100.

You're welcome thank you for your patches tying everything
together, it will be good to have this all finally working
OOTB.

> Le 02/10/2017 à 17:23, Hans de Goede a écrit :
>> 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);
> 
> You should also return 0 in bcm_resource(), otherwise the resources list is empty, ending up with "BCM irq: -22" trace in dmesg.

Right, good one. I did not notice that as on the device I was testing with
the IRQ is specified in the DSDT as a GpioInt rather as an Interrupt.

Fixed for v2 of this series.

Regards,

Hans

^ permalink raw reply

* [PATCH] serial: sh-sci: Use of_device_get_match_data() helper
From: Geert Uytterhoeven @ 2017-10-04 12:21 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jiri Slaby, linux-serial, linux-renesas-soc, Geert Uytterhoeven

Use the of_device_get_match_data() helper instead of open coding.
Note that when used with DT, there's always a valid match.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/tty/serial/sh-sci.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 784dd42002eadbeb..95b12eefcb7ae906 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -40,6 +40,7 @@
 #include <linux/module.h>
 #include <linux/mm.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/scatterlist.h>
@@ -3044,17 +3045,15 @@ static struct plat_sci_port *sci_parse_dt(struct platform_device *pdev,
 					  unsigned int *dev_id)
 {
 	struct device_node *np = pdev->dev.of_node;
-	const struct of_device_id *match;
 	struct plat_sci_port *p;
 	struct sci_port *sp;
+	const void *data;
 	int id;
 
 	if (!IS_ENABLED(CONFIG_OF) || !np)
 		return NULL;
 
-	match = of_match_node(of_sci_match, np);
-	if (!match)
-		return NULL;
+	data = of_device_get_match_data(&pdev->dev);
 
 	p = devm_kzalloc(&pdev->dev, sizeof(struct plat_sci_port), GFP_KERNEL);
 	if (!p)
@@ -3070,8 +3069,8 @@ static struct plat_sci_port *sci_parse_dt(struct platform_device *pdev,
 	sp = &sci_ports[id];
 	*dev_id = id;
 
-	p->type = SCI_OF_TYPE(match->data);
-	p->regtype = SCI_OF_REGTYPE(match->data);
+	p->type = SCI_OF_TYPE(data);
+	p->regtype = SCI_OF_REGTYPE(data);
 
 	sp->has_rtscts = of_property_read_bool(np, "uart-has-rtscts");
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH 2/2] ACPI / scan: Fix enumeration for special UART devices
From: Frédéric Danis @ 2017-10-04  8:51 UTC (permalink / raw)
  To: robh, marcel, sre, loic.poulain, johan, lukas, hdegoede
  Cc: linux-bluetooth, linux-serial, linux-acpi, frederic.danis.oss
In-Reply-To: <1507107090-15992-1-git-send-email-frederic.danis.oss@gmail.com>

UART devices is expected to be enumerated by SerDev subsystem.

During ACPI scan, serial devices behind SPI, I2C or UART buses are not
enumerated, allowing them to be enumerated by their respective parents.

Rename *spi_i2c_slave* to *serial_bus_slave* as this will be used for serial
devices on serial buses (SPI, I2C or UART).

On Macs an empty ResourceTemplate is returned for uart slaves.
Instead the device properties "baud", "parity", "dataBits", "stopBits" are
provided. Add a check for "baud" in acpi_is_serial_bus_slave().

Signed-off-by: Frédéric Danis <frederic.danis.oss@gmail.com>
---
 drivers/acpi/scan.c     | 37 +++++++++++++++++--------------------
 include/acpi/acpi_bus.h |  2 +-
 2 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 602f8ff..860b698 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1505,41 +1505,38 @@ static void acpi_init_coherency(struct acpi_device *adev)
 	adev->flags.coherent_dma = cca;
 }
 
-static int acpi_check_spi_i2c_slave(struct acpi_resource *ares, void *data)
+static int acpi_check_serial_bus_slave(struct acpi_resource *ares, void *data)
 {
-	bool *is_spi_i2c_slave_p = data;
+	bool *is_serial_bus_slave_p = data;
 
 	if (ares->type != ACPI_RESOURCE_TYPE_SERIAL_BUS)
 		return 1;
 
-	/*
-	 * devices that are connected to UART still need to be enumerated to
-	 * platform bus
-	 */
-	if (ares->data.common_serial_bus.type != ACPI_RESOURCE_SERIAL_TYPE_UART)
-		*is_spi_i2c_slave_p = true;
+	*is_serial_bus_slave_p = true;
 
 	 /* no need to do more checking */
 	return -1;
 }
 
-static bool acpi_is_spi_i2c_slave(struct acpi_device *device)
+static bool acpi_is_serial_bus_slave(struct acpi_device *device)
 {
 	struct list_head resource_list;
-	bool is_spi_i2c_slave = false;
+	bool is_serial_bus_slave = false;
 
 	/* Macs use device properties in lieu of _CRS resources */
 	if (x86_apple_machine &&
 	    (fwnode_property_present(&device->fwnode, "spiSclkPeriod") ||
-	     fwnode_property_present(&device->fwnode, "i2cAddress")))
+	     fwnode_property_present(&device->fwnode, "i2cAddress") ||
+	     fwnode_property_present(&device->fwnode, "baud")))
 		return true;
 
 	INIT_LIST_HEAD(&resource_list);
-	acpi_dev_get_resources(device, &resource_list, acpi_check_spi_i2c_slave,
-			       &is_spi_i2c_slave);
+	acpi_dev_get_resources(device, &resource_list,
+			       acpi_check_serial_bus_slave,
+			       &is_serial_bus_slave);
 	acpi_dev_free_resource_list(&resource_list);
 
-	return is_spi_i2c_slave;
+	return is_serial_bus_slave;
 }
 
 void acpi_init_device_object(struct acpi_device *device, acpi_handle handle,
@@ -1557,7 +1554,7 @@ void acpi_init_device_object(struct acpi_device *device, acpi_handle handle,
 	acpi_bus_get_flags(device);
 	device->flags.match_driver = false;
 	device->flags.initialized = true;
-	device->flags.spi_i2c_slave = acpi_is_spi_i2c_slave(device);
+	device->flags.serial_bus_slave = acpi_is_serial_bus_slave(device);
 	acpi_device_clear_enumerated(device);
 	device_initialize(&device->dev);
 	dev_set_uevent_suppress(&device->dev, true);
@@ -1841,10 +1838,10 @@ static acpi_status acpi_bus_check_add(acpi_handle handle, u32 lvl_not_used,
 static void acpi_default_enumeration(struct acpi_device *device)
 {
 	/*
-	 * Do not enumerate SPI/I2C slaves as they will be enumerated by their
-	 * respective parents.
+	 * Do not enumerate SPI/I2C/UART slaves as they will be enumerated by
+	 * their respective parents.
 	 */
-	if (!device->flags.spi_i2c_slave) {
+	if (!device->flags.serial_bus_slave) {
 		acpi_create_platform_device(device, NULL);
 		acpi_device_set_enumerated(device);
 	} else {
@@ -1941,7 +1938,7 @@ static void acpi_bus_attach(struct acpi_device *device)
 		return;
 
 	device->flags.match_driver = true;
-	if (ret > 0 && !device->flags.spi_i2c_slave) {
+	if (ret > 0 && !device->flags.serial_bus_slave) {
 		acpi_device_set_enumerated(device);
 		goto ok;
 	}
@@ -1950,7 +1947,7 @@ static void acpi_bus_attach(struct acpi_device *device)
 	if (ret < 0)
 		return;
 
-	if (!device->pnp.type.platform_id && !device->flags.spi_i2c_slave)
+	if (!device->pnp.type.platform_id && !device->flags.serial_bus_slave)
 		acpi_device_set_enumerated(device);
 	else
 		acpi_default_enumeration(device);
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index fa15052..f849be2 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -211,7 +211,7 @@ struct acpi_device_flags {
 	u32 of_compatible_ok:1;
 	u32 coherent_dma:1;
 	u32 cca_seen:1;
-	u32 spi_i2c_slave:1;
+	u32 serial_bus_slave:1;
 	u32 reserved:19;
 };
 
-- 
2.7.4


^ permalink raw reply related

* [PATCH 1/2] serdev: Add ACPI support
From: Frédéric Danis @ 2017-10-04  8:51 UTC (permalink / raw)
  To: robh-DgEjT+Ai2ygdnm+yROfE0A, marcel-kz+m5ild9QBg9hUCZPvPmw,
	sre-DgEjT+Ai2ygdnm+yROfE0A, loic.poulain-Re5JQEeQqe8AvxtiuMwx3w,
	johan-DgEjT+Ai2ygdnm+yROfE0A, lukas-JFq808J9C/izQB+pC5nmwQ,
	hdegoede-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	frederic.danis.oss-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1507107090-15992-1-git-send-email-frederic.danis.oss-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

This patch allows SerDev module to manage serial devices declared as
attached to an UART in ACPI table.

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.

Signed-off-by: Frédéric Danis <frederic.danis.oss-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/tty/serdev/core.c | 99 ++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 94 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index c68fb3a..104777d 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -14,6 +14,7 @@
  * GNU General Public License for more details.
  */
 
+#include <linux/acpi.h>
 #include <linux/errno.h>
 #include <linux/idr.h>
 #include <linux/kernel.h>
@@ -49,13 +50,22 @@ static const struct device_type serdev_ctrl_type = {
 
 static int serdev_device_match(struct device *dev, struct device_driver *drv)
 {
-	/* TODO: ACPI and platform matching */
+	/* TODO: platform matching */
+	if (acpi_driver_match_device(dev, drv))
+		return 1;
+
 	return of_driver_match_device(dev, drv);
 }
 
 static int serdev_uevent(struct device *dev, struct kobj_uevent_env *env)
 {
-	/* TODO: ACPI and platform modalias */
+	int rc;
+
+	/* TODO: platform modalias */
+	rc = acpi_device_uevent_modalias(dev, env);
+	if (rc != -ENODEV)
+		return rc;
+
 	return of_device_uevent_modalias(dev, env);
 }
 
@@ -260,6 +270,12 @@ static int serdev_drv_remove(struct device *dev)
 static ssize_t modalias_show(struct device *dev,
 			     struct device_attribute *attr, char *buf)
 {
+	int len;
+
+	len = acpi_device_modalias(dev, buf, PAGE_SIZE - 1);
+	if (len != -ENODEV)
+		return len;
+
 	return of_device_modalias(dev, buf, PAGE_SIZE);
 }
 DEVICE_ATTR_RO(modalias);
@@ -385,6 +401,74 @@ static int of_serdev_register_devices(struct serdev_controller *ctrl)
 	return 0;
 }
 
+#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;
+}
+#else
+static inline int acpi_serdev_register_devices(struct serdev_controller *ctlr)
+{
+	return -ENODEV;
+}
+#endif /* CONFIG_ACPI */
+
 /**
  * serdev_controller_add() - Add an serdev controller
  * @ctrl:	controller to be registered.
@@ -394,7 +478,7 @@ static int of_serdev_register_devices(struct serdev_controller *ctrl)
  */
 int serdev_controller_add(struct serdev_controller *ctrl)
 {
-	int ret;
+	int ret_of, ret_acpi, ret;
 
 	/* Can't register until after driver model init */
 	if (WARN_ON(!is_registered))
@@ -404,9 +488,14 @@ int serdev_controller_add(struct serdev_controller *ctrl)
 	if (ret)
 		return ret;
 
-	ret = of_serdev_register_devices(ctrl);
-	if (ret)
+	ret_of = of_serdev_register_devices(ctrl);
+	ret_acpi = acpi_serdev_register_devices(ctrl);
+	if (ret_of && ret_acpi) {
+		dev_dbg(&ctrl->dev, "serdev%d no devices registered: of:%d acpi:%d\n",
+			ctrl->nr, ret_of, ret_acpi);
+		ret = -ENODEV;
 		goto out_dev_del;
+	}
 
 	dev_dbg(&ctrl->dev, "serdev%d registered: dev:%p\n",
 		ctrl->nr, &ctrl->dev);
-- 
2.7.4

^ permalink raw reply related

* [PATCH 0/2] ACPI serdev support
From: Frédéric Danis @ 2017-10-04  8:51 UTC (permalink / raw)
  To: robh, marcel, sre, loic.poulain, johan, lukas, hdegoede
  Cc: linux-bluetooth, linux-serial, linux-acpi, frederic.danis.oss

Add ACPI support for serial attached devices.

Currently, serial devices are not set as enumerated during ACPI scan for SPI
or i2c buses (but not for UART). This should also be done for UART serial
devices.
I renamed *spi_i2c_slave* to *serial_bus_slave* to reflect this.

For hci_bcm, this needs Hans de Goede's "Bluetooth: hci_bcm: Add (runtime)pm
support to the serdev drv" patches to work.

Tested on T100TA with Broadcom BCM2E39.

Since RFC:
  - Add or reword commit messages
  - Rename *serial_slave* to *serial_bus_slave*
  - Add specific check for Apple in acpi_is_serial_bus_slave(), thanks to
    Lukas Wunner
  - Update comment in acpi_default_enumeration()
  - Remove patch 3 "Bluetooth: hci_bcm: Add ACPI serdev support for BCM2E39"
    in favor of patches from Hans de Goede

Frédéric Danis (2):
  serdev: Add ACPI support
  ACPI / scan: Fix enumeration for special UART devices

 drivers/acpi/scan.c       | 37 ++++++++----------
 drivers/tty/serdev/core.c | 99 ++++++++++++++++++++++++++++++++++++++++++++---
 include/acpi/acpi_bus.h   |  2 +-
 3 files changed, 112 insertions(+), 26 deletions(-)

-- 
2.7.4


^ permalink raw reply

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

On Fri, Sep 29, 2017 at 03:08:54PM +0200, Ulrich Hecht 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>
> ---
>  drivers/tty/serial/sh-sci.c | 62 +++++++++++++++++++++++++++++++++++++++++++--
>  drivers/tty/serial/sh-sci.h |  4 +++
>  2 files changed, 64 insertions(+), 2 deletions(-)

Adding random sysfs files for random serial ports isn't good, especially
if you do not document them in Documentation/ABI.

I don't want to see this, but if you write sysfs files in the future,
here's some review comments:

> +static DEVICE_ATTR(rx_sampling_point, 0644,
> +		   rx_sampling_point_show,
> +		   rx_sampling_point_store);

DEVICE_ATTR_RW()

> +	if (port->port.type == PORT_HSCIF) {
> +		sysfs_remove_file(&dev->dev.kobj,
> +				  &dev_attr_rx_sampling_point.attr);

No driver code should ever call sysfs functions, this should be either:
	device_create_file()
or properly use an attribute group.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH] serial: imx: Correct comment imx_flush_buffer()
From: Greg Kroah-Hartman @ 2017-10-03 18:27 UTC (permalink / raw)
  To: Martyn Welch; +Cc: Jiri Slaby, linux-serial, linux-kernel
In-Reply-To: <1506676939-20940-1-git-send-email-martyn.welch@collabora.co.uk>

On Fri, Sep 29, 2017 at 10:22:19AM +0100, Martyn Welch wrote:
> The comment in imx_flush_buffer() states that the state of 4 registers
> are to be saved/restored, then only saves and restores 3 registers. The
> missing register (UBRC) is read only and thus can't be restored.
> 
> Update the comment to reflect reality.

Always run checkpatch.pl so you don't get grumpy maintainers telling you
to run checkpatch.pl :(

^ permalink raw reply

* Re: [PATCH v2] Use RX_BUF_SIZE to set size of RX buffer
From: Greg Kroah-Hartman @ 2017-10-03 18:24 UTC (permalink / raw)
  To: Martyn Welch
  Cc: Jiri Slaby, linux-serial, linux-kernel, Uwe Kleine-König
In-Reply-To: <1506593260-14457-1-git-send-email-martyn.welch@collabora.co.uk>

On Thu, Sep 28, 2017 at 11:07:40AM +0100, Martyn Welch wrote:
> The imx serial driver uses PAGE_SIZE when allocating rx_buf, but then
> uses RX_BUF_SIZE (which is currently defined as PAGE_SIZE) to describe
> the length of the buffer when initialising the scatter gather list.
> 
> In order to ensure that this stays consistent, use RX_BUF_SIZE in both
> locations.
> 
> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
> Acked-by: Uwe Kleine-König <u.kleine-könig@pengtronix.de>
> ---
> 
> v2: Add missing SoB and Uwe's Acked-by.

Your subject is missing "serial: imx:" :(

I'll go fix it up...

^ permalink raw reply

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

Hi Hans,

First of all, many thanks for your work on this, it helped me a lot to 
get full Bluetooth on my Asus T100.

Le 02/10/2017 à 17:23, Hans de Goede a écrit :
> 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);

You should also return 0 in bcm_resource(), otherwise the resources list 
is empty, ending up with "BCM irq: -22" trace in dmesg.

Regards,

Frédéric Danis

^ permalink raw reply

* [GIT PULL] TTY/Serial fixes for 4.14-rc4
From: Greg KH @ 2017-10-03 12:20 UTC (permalink / raw)
  To: Linus Torvalds, Jiri Slaby
  Cc: Stephen Rothwell, Andrew Morton, linux-kernel, linux-serial

The following changes since commit 2bd6bf03f4c1c59381d62c61d03f6cc3fe71f66e:

  Linux 4.14-rc1 (2017-09-16 15:47:51 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/ tags/tty-4.14-rc4

for you to fetch changes up to c91261437985d481c472639d4397931d77f5d4e9:

  serial: sccnxp: Fix error handling in sccnxp_probe() (2017-09-18 18:19:21 +0200)

----------------------------------------------------------------
TTY/Serial fixes for 4.14-rc4

Here are a small number (5) of patches for some reported TTY and serial
issues.  Nothing major, a documentation update, timing fix, error
handling fix, name reporting fix, and a timeout issue resolved.

All of these have been in linux-next for a while with no reported
issues.

Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

----------------------------------------------------------------
Alexey Khoroshilov (1):
      serial: sccnxp: Fix error handling in sccnxp_probe()

Fugang Duan (1):
      tty: serial: lpuart: avoid report NULL interrupt

Jiri Slaby (1):
      mxser: fix timeout calculation for low rates

Russell Enderby (1):
      serial: bcm63xx: fix timing issue.

Sergei Shtylyov (1):
      serial: sh-sci: document R8A77970 bindings

 .../bindings/serial/renesas,sci-serial.txt         |  2 ++
 drivers/tty/mxser.c                                | 16 ++++++---
 drivers/tty/serial/bcm63xx_uart.c                  |  5 +++
 drivers/tty/serial/fsl_lpuart.c                    | 40 +++++++++-------------
 drivers/tty/serial/sccnxp.c                        | 13 +++++--
 5 files changed, 46 insertions(+), 30 deletions(-)

^ permalink raw reply

* [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