Linux Serial subsystem development
 help / color / mirror / Atom feed
* [PATCH v2 5/6] gpio-exar/8250-exar: Make set of exported GPIOs configurable
From: Jan Kiszka @ 2017-05-18 14:59 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot
  Cc: Linux Kernel Mailing List, linux-serial, linux-gpio,
	Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
In-Reply-To: <cover.1495119548.git.jan.kiszka@siemens.com>

On the SIMATIC, IOT2040 only a single pin is exportable as GPIO, the
rest is required to operate the UART. To allow modeling this case,
expand the platform device data structure to specify a (consecutive) pin
subset for exporting by the gpio-exar driver.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/gpio/gpio-exar.c                | 28 +++++++++++++++++++---------
 drivers/tty/serial/8250/8250_exar.c     |  7 +++++--
 include/linux/platform_data/gpio-exar.h |  3 +++
 3 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
index d8b6296c11de..e0698a1ae012 100644
--- a/drivers/gpio/gpio-exar.c
+++ b/drivers/gpio/gpio-exar.c
@@ -32,6 +32,7 @@ struct exar_gpio_chip {
 	int index;
 	void __iomem *regs;
 	char name[20];
+	unsigned int first_gpio;
 };
 
 static void exar_update(struct gpio_chip *chip, unsigned int reg, int val,
@@ -52,9 +53,11 @@ static void exar_update(struct gpio_chip *chip, unsigned int reg, int val,
 static int exar_set_direction(struct gpio_chip *chip, int direction,
 			      unsigned int offset)
 {
-	unsigned int bank = offset / 8;
-	unsigned int addr;
+	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
+	unsigned int bank, addr;
 
+	offset += exar_gpio->first_gpio;
+	bank = offset / 8;
 	addr = bank ? EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
 	exar_update(chip, addr, direction, offset % 8);
 	return 0;
@@ -74,10 +77,12 @@ static int exar_get(struct gpio_chip *chip, unsigned int reg)
 
 static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
 {
-	unsigned int bank = offset / 8;
-	unsigned int addr;
+	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
+	unsigned int bank, addr;
 	int val;
 
+	offset += exar_gpio->first_gpio;
+	bank = offset / 8;
 	addr = bank ? EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
 	val = exar_get(chip, addr) & BIT(offset % 8);
 
@@ -86,10 +91,12 @@ static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
 
 static int exar_get_value(struct gpio_chip *chip, unsigned int offset)
 {
-	unsigned int bank = offset / 8;
-	unsigned int addr;
+	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
+	unsigned int bank, addr;
 	int val;
 
+	offset += exar_gpio->first_gpio;
+	bank = offset / 8;
 	addr = bank ? EXAR_OFFSET_MPIOLVL_HI : EXAR_OFFSET_MPIOLVL_LO;
 	val = exar_get(chip, addr) & BIT(offset % 8);
 
@@ -99,9 +106,11 @@ static int exar_get_value(struct gpio_chip *chip, unsigned int offset)
 static void exar_set_value(struct gpio_chip *chip, unsigned int offset,
 			   int value)
 {
-	unsigned int bank = offset / 8;
-	unsigned int addr;
+	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
+	unsigned int bank, addr;
 
+	offset += exar_gpio->first_gpio;
+	bank = offset / 8;
 	addr = bank ? EXAR_OFFSET_MPIOLVL_HI : EXAR_OFFSET_MPIOLVL_LO;
 	exar_update(chip, addr, value, offset % 8);
 }
@@ -154,9 +163,10 @@ static int gpio_exar_probe(struct platform_device *pdev)
 	exar_gpio->gpio_chip.get = exar_get_value;
 	exar_gpio->gpio_chip.set = exar_set_value;
 	exar_gpio->gpio_chip.base = -1;
-	exar_gpio->gpio_chip.ngpio = 16;
+	exar_gpio->gpio_chip.ngpio = pdata->ngpio;
 	exar_gpio->regs = p;
 	exar_gpio->index = index;
+	exar_gpio->first_gpio = pdata->first_gpio;
 
 	ret = devm_gpiochip_add_data(&pdev->dev,
 				     &exar_gpio->gpio_chip, exar_gpio);
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index 0a806daaff8b..d9c52288d6c9 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -189,7 +189,8 @@ static void setup_gpio(u8 __iomem *p)
 }
 
 static void *
-xr17v35x_register_gpio(struct pci_dev *pcidev)
+xr17v35x_register_gpio(struct pci_dev *pcidev, unsigned int first_gpio,
+		       unsigned int ngpio)
 {
 	struct platform_device *pdev;
 	struct gpio_exar_pdata pdata;
@@ -199,6 +200,8 @@ xr17v35x_register_gpio(struct pci_dev *pcidev)
 		return NULL;
 
 	pdata.parent = pcidev;
+	pdata.first_gpio = first_gpio;
+	pdata.ngpio = ngpio;
 
 	if (platform_device_add_data(pdev, &pdata, sizeof(pdata)) < 0 ||
 	    platform_device_add(pdev) < 0) {
@@ -242,7 +245,7 @@ pci_xr17v35x_setup(struct exar8250 *priv, struct pci_dev *pcidev,
 		/* Setup Multipurpose Input/Output pins. */
 		setup_gpio(p);
 
-		port->port.private_data = xr17v35x_register_gpio(pcidev);
+		port->port.private_data = xr17v35x_register_gpio(pcidev, 0, 16);
 	}
 
 	return 0;
diff --git a/include/linux/platform_data/gpio-exar.h b/include/linux/platform_data/gpio-exar.h
index 1a13e9ddaf7d..1754f5a2842d 100644
--- a/include/linux/platform_data/gpio-exar.h
+++ b/include/linux/platform_data/gpio-exar.h
@@ -17,6 +17,9 @@ struct pci_dev;
 
 struct gpio_exar_pdata {
 	struct pci_dev *parent;
+
+	unsigned int first_gpio;
+	unsigned int ngpio;
 };
 
 #endif /* __GPIO_EXAR_PDATA_H */
-- 
2.12.0

^ permalink raw reply related

* [PATCH v2 4/6] gpio: exar: Fix reading of directions and values
From: Jan Kiszka @ 2017-05-18 14:59 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot
  Cc: Linux Kernel Mailing List, linux-serial, linux-gpio,
	Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
In-Reply-To: <cover.1495119548.git.jan.kiszka@siemens.com>

First, the logic for translating a register bit to the return code of
exar_get_direction and exar_get_value were wrong. And second, there was
a flip regarding the register bank in exar_get_direction.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/gpio/gpio-exar.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
index 4ed1f8bdeda7..d8b6296c11de 100644
--- a/drivers/gpio/gpio-exar.c
+++ b/drivers/gpio/gpio-exar.c
@@ -69,7 +69,7 @@ static int exar_get(struct gpio_chip *chip, unsigned int reg)
 	value = readb(exar_gpio->regs + reg);
 	mutex_unlock(&exar_gpio->lock);
 
-	return !!value;
+	return value;
 }
 
 static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
@@ -79,7 +79,7 @@ static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
 	int val;
 
 	addr = bank ? EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
-	val = exar_get(chip, addr) >> (offset % 8);
+	val = exar_get(chip, addr) & BIT(offset % 8);
 
 	return !!val;
 }
@@ -90,8 +90,8 @@ static int exar_get_value(struct gpio_chip *chip, unsigned int offset)
 	unsigned int addr;
 	int val;
 
-	addr = bank ? EXAR_OFFSET_MPIOLVL_LO : EXAR_OFFSET_MPIOLVL_HI;
-	val = exar_get(chip, addr) >> (offset % 8);
+	addr = bank ? EXAR_OFFSET_MPIOLVL_HI : EXAR_OFFSET_MPIOLVL_LO;
+	val = exar_get(chip, addr) & BIT(offset % 8);
 
 	return !!val;
 }
-- 
2.12.0


^ permalink raw reply related

* [PATCH v2 3/6] gpio: exar: Fix iomap request
From: Jan Kiszka @ 2017-05-18 14:59 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot
  Cc: Linux Kernel Mailing List, linux-serial, linux-gpio,
	Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
In-Reply-To: <cover.1495119548.git.jan.kiszka@siemens.com>

The UART driver already maps the resource for us. Trying to do this here
only fails and leaves us with a non-working device.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/gpio/gpio-exar.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
index b1dc5fff21ad..4ed1f8bdeda7 100644
--- a/drivers/gpio/gpio-exar.c
+++ b/drivers/gpio/gpio-exar.c
@@ -130,14 +130,10 @@ static int gpio_exar_probe(struct platform_device *pdev)
 		return -ENODEV;
 
 	/*
-	 * Map the pci device to get the register addresses.
-	 * We will need to read and write those registers to control
-	 * the GPIO pins.
-	 * Using managed functions will save us from unmaping on exit.
-	 * As the device is enabled using managed functions by the
-	 * UART driver we can also use managed functions here.
+	 * The UART driver must have mapped region 0 prior to registering this
+	 * device - use it.
 	 */
-	p = pcim_iomap(pcidev, 0, 0);
+	p = pcim_iomap_table(pcidev)[0];
 	if (!p)
 		return -ENOMEM;
 
-- 
2.12.0


^ permalink raw reply related

* [PATCH v2 2/6] gpio: exar: Allocate resources on behalf of the platform device
From: Jan Kiszka @ 2017-05-18 14:59 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot
  Cc: Linux Kernel Mailing List, linux-serial, linux-gpio,
	Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
In-Reply-To: <cover.1495119548.git.jan.kiszka@siemens.com>

Do not allocate resources on behalf of the parent device but on our own.
Otherwise, cleanup does not properly work if gpio-exar is removed but
not the parent device.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/gpio/gpio-exar.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
index d62e57513144..b1dc5fff21ad 100644
--- a/drivers/gpio/gpio-exar.c
+++ b/drivers/gpio/gpio-exar.c
@@ -141,7 +141,7 @@ static int gpio_exar_probe(struct platform_device *pdev)
 	if (!p)
 		return -ENOMEM;
 
-	exar_gpio = devm_kzalloc(&pcidev->dev, sizeof(*exar_gpio), GFP_KERNEL);
+	exar_gpio = devm_kzalloc(&pdev->dev, sizeof(*exar_gpio), GFP_KERNEL);
 	if (!exar_gpio)
 		return -ENOMEM;
 
@@ -162,7 +162,7 @@ static int gpio_exar_probe(struct platform_device *pdev)
 	exar_gpio->regs = p;
 	exar_gpio->index = index;
 
-	ret = devm_gpiochip_add_data(&pcidev->dev,
+	ret = devm_gpiochip_add_data(&pdev->dev,
 				     &exar_gpio->gpio_chip, exar_gpio);
 	if (ret)
 		goto err_destroy;
-- 
2.12.0


^ permalink raw reply related

* [PATCH v2 1/6] gpio: exar: Fix passing in of parent PCI device
From: Jan Kiszka @ 2017-05-18 14:59 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot
  Cc: Linux Kernel Mailing List, linux-serial, linux-gpio,
	Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
In-Reply-To: <cover.1495119548.git.jan.kiszka@siemens.com>

This fixes reloading of the GPIO driver for the same platform device
instance as created by the exar UART driver: First of all, the driver
sets drvdata to its own value during probing and does not restore the
original value on exit. But this won't help anyway as the core clears
drvdata after the driver left.

Use stable platform_data instead.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/gpio/gpio-exar.c                |  4 +++-
 drivers/tty/serial/8250/8250_exar.c     |  8 ++++++--
 include/linux/platform_data/gpio-exar.h | 22 ++++++++++++++++++++++
 3 files changed, 31 insertions(+), 3 deletions(-)
 create mode 100644 include/linux/platform_data/gpio-exar.h

diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
index 081076771217..d62e57513144 100644
--- a/drivers/gpio/gpio-exar.c
+++ b/drivers/gpio/gpio-exar.c
@@ -14,6 +14,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/pci.h>
+#include <linux/platform_data/gpio-exar.h>
 #include <linux/platform_device.h>
 
 #define EXAR_OFFSET_MPIOLVL_LO 0x90
@@ -119,7 +120,8 @@ static int exar_direction_input(struct gpio_chip *chip, unsigned int offset)
 
 static int gpio_exar_probe(struct platform_device *pdev)
 {
-	struct pci_dev *pcidev = platform_get_drvdata(pdev);
+	struct gpio_exar_pdata *pdata = pdev->dev.platform_data;
+	struct pci_dev *pcidev = pdata->parent;
 	struct exar_gpio_chip *exar_gpio;
 	void __iomem *p;
 	int index, ret;
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index b4fa585156c7..0a806daaff8b 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -13,6 +13,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/pci.h>
+#include <linux/platform_data/gpio-exar.h>
 #include <linux/serial_core.h>
 #include <linux/serial_reg.h>
 #include <linux/slab.h>
@@ -191,13 +192,16 @@ static void *
 xr17v35x_register_gpio(struct pci_dev *pcidev)
 {
 	struct platform_device *pdev;
+	struct gpio_exar_pdata pdata;
 
 	pdev = platform_device_alloc("gpio_exar", PLATFORM_DEVID_AUTO);
 	if (!pdev)
 		return NULL;
 
-	platform_set_drvdata(pdev, pcidev);
-	if (platform_device_add(pdev) < 0) {
+	pdata.parent = pcidev;
+
+	if (platform_device_add_data(pdev, &pdata, sizeof(pdata)) < 0 ||
+	    platform_device_add(pdev) < 0) {
 		platform_device_put(pdev);
 		return NULL;
 	}
diff --git a/include/linux/platform_data/gpio-exar.h b/include/linux/platform_data/gpio-exar.h
new file mode 100644
index 000000000000..1a13e9ddaf7d
--- /dev/null
+++ b/include/linux/platform_data/gpio-exar.h
@@ -0,0 +1,22 @@
+/*
+ * GPIO handling for Exar XR17V35X chip
+ *
+ * Copyright (c) 2017 Siemens AG
+ *
+ * Written by Jan Kiszka <jan.kiszka@siemens.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __GPIO_EXAR_PDATA_H
+#define __GPIO_EXAR_PDATA_H
+
+struct pci_dev;
+
+struct gpio_exar_pdata {
+	struct pci_dev *parent;
+};
+
+#endif /* __GPIO_EXAR_PDATA_H */
-- 
2.12.0

^ permalink raw reply related

* [PATCH v2 0/6] serial/gpio: exar: Fixes and support for IOT2000
From: Jan Kiszka @ 2017-05-18 14:59 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot
  Cc: Linux Kernel Mailing List, linux-serial, linux-gpio,
	Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger

This makes the gpio-exar driver usable, which was prevented by a number
of fatal bugs, and adds support for the SIMATIC IOT2040 to the 8250-exar
driver and, indirectly, to gpio-exar as well. It's a cross-subsystem
series, so I'm also cross-posting to the serial and gpio lists.

Changes in v2:
 - dropped already merged patches
 - switched to platform_data structure for exar <-> gpio driver
   communication
 - use dmi_check_system() to express platform match
 - adjusted bit logic style in exar_get_direction/value

Jan

Jan Kiszka (6):
  gpio: exar: Fix passing in of parent PCI device
  gpio: exar: Allocate resources on behalf of the platform device
  gpio: exar: Fix iomap request
  gpio: exar: Fix reading of directions and values
  gpio-exar/8250-exar: Make set of exported GPIOs configurable
  serial: exar: Add support for IOT2040 device

 drivers/gpio/gpio-exar.c                |  54 +++++++-----
 drivers/tty/serial/8250/8250_exar.c     | 151 ++++++++++++++++++++++++++++++--
 include/linux/platform_data/gpio-exar.h |  25 ++++++
 3 files changed, 198 insertions(+), 32 deletions(-)
 create mode 100644 include/linux/platform_data/gpio-exar.h

-- 
2.12.0

^ permalink raw reply

* Re: [PATCH 3/4] tty/serdev: add serdev registration interface
From: Greg Kroah-Hartman @ 2017-05-18 14:43 UTC (permalink / raw)
  To: Johan Hovold; +Cc: Rob Herring, Jiri Slaby, linux-serial, linux-kernel
In-Reply-To: <20170411170731.4085-4-johan@kernel.org>

On Tue, Apr 11, 2017 at 07:07:30PM +0200, Johan Hovold wrote:
> Add a new interface for registering a serdev controller and clients, and
> a helper function to deregister serdev devices (or a tty device) that
> were previously registered using the new interface.
> 
> Once every driver currently using the tty_port_register_device() helpers
> have been vetted and converted to use the new serdev registration
> interface (at least for deregistration), we can move serdev registration
> to the current helpers and get rid of the serdev-specific functions.
> 
> Signed-off-by: Johan Hovold <johan@kernel.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
>  drivers/tty/serdev/serdev-ttyport.c |  6 ++-
>  drivers/tty/tty_port.c              | 76 +++++++++++++++++++++++++++++++++++++
>  include/linux/serdev.h              |  7 +++-
>  include/linux/tty.h                 |  9 +++++
>  4 files changed, 94 insertions(+), 4 deletions(-)

This patch doesn't apply, and should it really be a 4.12-final patch?

I've dropped this one, and the 4th as well, from my queue.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH v5 16/17] tty: serdev-ttyport: return actual baudrate from ttyport_set_baudrate
From: Greg Kroah-Hartman @ 2017-05-18 14:19 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: Rob Herring, David S. Miller, Mark Rutland, Jiri Slaby,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Lino Sanfilippo, Jakub Kicinski
In-Reply-To: <d5533816-5d87-3772-19c0-5e57e519a3f1-eS4NqCHxEME@public.gmane.org>

On Wed, May 17, 2017 at 01:58:05PM +0200, Stefan Wahren wrote:
> Hi Greg,
> 
> Am 10.05.2017 um 10:53 schrieb Stefan Wahren:
> > Instead of returning the requested baudrate, we better return the
> > actual one because it isn't always the same.
> >
> > Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> > Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > ---
> >  drivers/tty/serdev/serdev-ttyport.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
> > index 487c88f..2cfdf34 100644
> > --- a/drivers/tty/serdev/serdev-ttyport.c
> > +++ b/drivers/tty/serdev/serdev-ttyport.c
> > @@ -151,7 +151,7 @@ static unsigned int ttyport_set_baudrate(struct serdev_controller *ctrl, unsigne
> >  
> >  	/* tty_set_termios() return not checked as it is always 0 */
> >  	tty_set_termios(tty, &ktermios);
> > -	return speed;
> > +	return ktermios.c_ospeed;
> >  }
> >  
> >  static void ttyport_set_flow_control(struct serdev_controller *ctrl, bool enable)
> 
> there is no dependency to this series. I would be happy if this could be
> applied, so i don't have to carry it for the next version.

Ok, I'll take it now.

thanks,

greg k-h
--
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

* Re: [PATCH-stable] serial: samsung: Bring back removed variable to fix compilation
From: Krzysztof Kozlowski @ 2017-05-18 13:16 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Krzysztof Kozlowski, Jiri Slaby, linux-serial, linux-kernel,
	stable
In-Reply-To: <20170518131440.GA14129@kroah.com>

On Thu, May 18, 2017 at 3:14 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, May 18, 2017 at 02:59:32PM +0200, Krzysztof Kozlowski wrote:
>> On Thu, May 18, 2017 at 2:31 PM, Krzysztof Kozlowski
>> <kozik.server@gmail.com> wrote:
>>
>> I need to fixup the from field. It should be krzk@kernel.org.
>
> I've merged this with the original patch so nothing breaks...

Great, thanks!

Krzysztof

^ permalink raw reply

* Re: [PATCH-stable] serial: samsung: Bring back removed variable to fix compilation
From: Greg Kroah-Hartman @ 2017-05-18 13:14 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Krzysztof Kozlowski, Jiri Slaby, linux-serial, linux-kernel,
	stable
In-Reply-To: <CAJKOXPdEQ-JSSZK1PA4xc1f7NWdKNyaeQ3qyjEdW7ttTvTF4og@mail.gmail.com>

On Thu, May 18, 2017 at 02:59:32PM +0200, Krzysztof Kozlowski wrote:
> On Thu, May 18, 2017 at 2:31 PM, Krzysztof Kozlowski
> <kozik.server@gmail.com> wrote:
> 
> I need to fixup the from field. It should be krzk@kernel.org.

I've merged this with the original patch so nothing breaks...

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH-stable] serial: samsung: Bring back removed variable to fix compilation
From: Krzysztof Kozlowski @ 2017-05-18 12:59 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel,
	stable
In-Reply-To: <1495110677-31174-1-git-send-email-krzk@kernel.org>

On Thu, May 18, 2017 at 2:31 PM, Krzysztof Kozlowski
<kozik.server@gmail.com> wrote:

I need to fixup the from field. It should be krzk@kernel.org.

Krzysztof


> By mistake commit 500fcc08a32b ("serial: samsung: Add missing checks for
> dma_map_single failure") removes a local variable which is still used by
> code.  After backporting the commit to stable, compilation fails:
>
>         drivers/tty/serial/samsung.c: In function ‘s3c24xx_serial_request_dma’:
>         drivers/tty/serial/samsung.c:909:45: error: ‘flags’ undeclared (first use in this function)
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>
> ---
>
> Fix for stable-rc commit:
> bc3ac2299c4873cdc7f9bf3c867c23e9714a036f
> ---
>  drivers/tty/serial/samsung.c | 1 +
>  1 file changed, 1 insertion(+)

^ permalink raw reply

* [PATCH-stable] serial: samsung: Bring back removed variable to fix compilation
From: Krzysztof Kozlowski @ 2017-05-18 12:31 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel,
	stable
  Cc: Krzysztof Kozlowski

By mistake commit 500fcc08a32b ("serial: samsung: Add missing checks for
dma_map_single failure") removes a local variable which is still used by
code.  After backporting the commit to stable, compilation fails:

	drivers/tty/serial/samsung.c: In function ‘s3c24xx_serial_request_dma’:
	drivers/tty/serial/samsung.c:909:45: error: ‘flags’ undeclared (first use in this function)

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

---

Fix for stable-rc commit:
bc3ac2299c4873cdc7f9bf3c867c23e9714a036f
---
 drivers/tty/serial/samsung.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index ca0bcd7fd61f..43c84c9bb904 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -859,6 +859,7 @@ static void s3c24xx_serial_break_ctl(struct uart_port *port, int break_state)
 static int s3c24xx_serial_request_dma(struct s3c24xx_uart_port *p)
 {
 	struct s3c24xx_uart_dma	*dma = p->dma;
+	unsigned long flags;
 	int ret;
 
 	/* Default slave configuration parameters */
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH 8/8] serial: exar: Add support for IOT2040 device
From: Andy Shevchenko @ 2017-05-18 10:56 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot,
	Linux Kernel Mailing List, linux-serial@vger.kernel.org,
	linux-gpio@vger.kernel.org, Sudip Mukherjee, Sascha Weisenberger
In-Reply-To: <1ee7fc16-5bba-7be6-57b3-c6efaf9a1c6f@siemens.com>

On Thu, May 18, 2017 at 1:37 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2017-05-18 12:17, Andy Shevchenko wrote:
>> On Thu, May 18, 2017 at 8:06 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> On 2017-05-13 15:54, Andy Shevchenko wrote:

>>>> Please, refactor that using properly formed DMI structrure and use its
>>>> callback and driver data facilities/
>>>
>>> Could you point to a specific example? The callback of the dmi_system_id
>>> structure is not designed to handle device initializations like this one
>>> - not to speak of having those two different initialization points here.
>>
>> Sure.
>>
>> drivers/platform/x86/dell-laptop.c:
>>
>> static const struct dmi_system_id dell_quirks[] __initconst = {
>>         {
>>                 .callback = dmi_matched,
>>                 .ident = "Dell Vostro V130",
>>                 .matches = {
>>                         DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>>                         DMI_MATCH(DMI_PRODUCT_NAME, "Vostro V130"),
>>                 },
>>                 .driver_data = &quirk_dell_vostro_v130,
>>         },
>>
>>
>> (just in case, dmi_matched() is part of that module)
>>
>
> OK, but that would basically replace the two lines for matching against
> the DMI values with the structure above. We would still need the
> existing hooks at the two points where we have them right now.

Right.

> Doesn't
> look to me as if the code would become clearer or better separated or
> easier extensible (the latter being hard anyway without knowing any
> potential third special case).

stmmac, that you chose as an example, should go that way in the first
place because it was my though behind which is on-to-one to yours
above.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH 8/8] serial: exar: Add support for IOT2040 device
From: Jan Kiszka @ 2017-05-18 10:37 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot,
	Linux Kernel Mailing List, linux-serial@vger.kernel.org,
	linux-gpio@vger.kernel.org, Sudip Mukherjee, Sascha Weisenberger
In-Reply-To: <CAHp75VcKZumhYqc9SW_JM+8ecAEfi7mWbr6=d7kFVeEPA1_LkA@mail.gmail.com>

On 2017-05-18 12:17, Andy Shevchenko wrote:
> On Thu, May 18, 2017 at 8:06 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2017-05-13 15:54, Andy Shevchenko wrote:
>>> On Sat, May 13, 2017 at 10:29 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> This implements the setup of RS232 and the switch-over to RS485 or RS422
>>>> for the Siemens IOT2040. That uses an EXAR XR17V352 with external logic
>>>> to switch between the different modes. The external logic is controlled
>>>> via MPIO pins of the EXAR controller.
>>>>
>>>> Only pin 10 can be exported as GPIO on the IOT2040. It is connected to
>>>> an LED.
>>>>
>>>> As the XR17V352 used on the IOT2040 is not equipped with an external
>>>> EEPROM, it cannot present itself as IOT2040-variant via subvendor/
>>>> subdevice IDs. Thus, we have to check via DMI for the target platform.
>>>>
>>>> Co-developed with Sascha Weisenberger.
>>>>
>>>
>>> Please, refactor that using properly formed DMI structrure and use its
>>> callback and driver data facilities/
>>
>> Could you point to a specific example? The callback of the dmi_system_id
>> structure is not designed to handle device initializations like this one
>> - not to speak of having those two different initialization points here.
> 
> Sure.
> 
> drivers/platform/x86/dell-laptop.c:
> 
> static const struct dmi_system_id dell_quirks[] __initconst = {
>         {
>                 .callback = dmi_matched,
>                 .ident = "Dell Vostro V130",
>                 .matches = {
>                         DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>                         DMI_MATCH(DMI_PRODUCT_NAME, "Vostro V130"),
>                 },
>                 .driver_data = &quirk_dell_vostro_v130,
>         },
> 
> 
> (just in case, dmi_matched() is part of that module)
> 

OK, but that would basically replace the two lines for matching against
the DMI values with the structure above. We would still need the
existing hooks at the two points where we have them right now. Doesn't
look to me as if the code would become clearer or better separated or
easier extensible (the latter being hard anyway without knowing any
potential third special case).

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

^ permalink raw reply

* Re: [PATCH 5/8] gpio: exar: Fix reading of directions and values
From: Jan Kiszka @ 2017-05-18 10:28 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot,
	Linux Kernel Mailing List, linux-serial@vger.kernel.org,
	linux-gpio@vger.kernel.org, Sudip Mukherjee, Sascha Weisenberger
In-Reply-To: <CAHp75VdsL_bT-HN3yHxNwQTMfJnfDc1EPEmn+ARZSvZDts7W6g@mail.gmail.com>

On 2017-05-18 12:23, Andy Shevchenko wrote:
> On Thu, May 18, 2017 at 1:16 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2017-05-18 12:11, Andy Shevchenko wrote:
>>> On Thu, May 18, 2017 at 8:20 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> 
>>> Full picture:
>>>
>>>  val = exar_get(chip, addr) & BIT(offset % 8);
>>>
>>>  return !!val;
>>>
>>> How it could be non-(1 or 0)?
>>>
>>
>> Right - but what is the point of that other style?
> 
> gpio-exar.c is just 4th module which is using it, OTOH the rest of
> GPIO drivers are using return !!val style.
> 

OK, consistency is valid point. Will adjust.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

^ permalink raw reply

* Re: [PATCH 5/8] gpio: exar: Fix reading of directions and values
From: Andy Shevchenko @ 2017-05-18 10:23 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot,
	Linux Kernel Mailing List, linux-serial@vger.kernel.org,
	linux-gpio@vger.kernel.org, Sudip Mukherjee, Sascha Weisenberger
In-Reply-To: <160112c1-c80b-5b1b-6e54-4b3c22f554b6@siemens.com>

On Thu, May 18, 2017 at 1:16 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2017-05-18 12:11, Andy Shevchenko wrote:
>> On Thu, May 18, 2017 at 8:20 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:

>> Full picture:
>>
>>  val = exar_get(chip, addr) & BIT(offset % 8);
>>
>>  return !!val;
>>
>> How it could be non-(1 or 0)?
>>
>
> Right - but what is the point of that other style?

gpio-exar.c is just 4th module which is using it, OTOH the rest of
GPIO drivers are using return !!val style.


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH 8/8] serial: exar: Add support for IOT2040 device
From: Andy Shevchenko @ 2017-05-18 10:17 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot,
	Linux Kernel Mailing List, linux-serial@vger.kernel.org,
	linux-gpio@vger.kernel.org, Sudip Mukherjee, Sascha Weisenberger
In-Reply-To: <9c3fa989-b137-2935-23cd-d9c3a91c6ce6@siemens.com>

On Thu, May 18, 2017 at 8:06 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2017-05-13 15:54, Andy Shevchenko wrote:
>> On Sat, May 13, 2017 at 10:29 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> This implements the setup of RS232 and the switch-over to RS485 or RS422
>>> for the Siemens IOT2040. That uses an EXAR XR17V352 with external logic
>>> to switch between the different modes. The external logic is controlled
>>> via MPIO pins of the EXAR controller.
>>>
>>> Only pin 10 can be exported as GPIO on the IOT2040. It is connected to
>>> an LED.
>>>
>>> As the XR17V352 used on the IOT2040 is not equipped with an external
>>> EEPROM, it cannot present itself as IOT2040-variant via subvendor/
>>> subdevice IDs. Thus, we have to check via DMI for the target platform.
>>>
>>> Co-developed with Sascha Weisenberger.
>>>
>>
>> Please, refactor that using properly formed DMI structrure and use its
>> callback and driver data facilities/
>
> Could you point to a specific example? The callback of the dmi_system_id
> structure is not designed to handle device initializations like this one
> - not to speak of having those two different initialization points here.

Sure.

drivers/platform/x86/dell-laptop.c:

static const struct dmi_system_id dell_quirks[] __initconst = {
        {
                .callback = dmi_matched,
                .ident = "Dell Vostro V130",
                .matches = {
                        DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
                        DMI_MATCH(DMI_PRODUCT_NAME, "Vostro V130"),
                },
                .driver_data = &quirk_dell_vostro_v130,
        },


(just in case, dmi_matched() is part of that module)

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH 5/8] gpio: exar: Fix reading of directions and values
From: Jan Kiszka @ 2017-05-18 10:16 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot,
	Linux Kernel Mailing List, linux-serial@vger.kernel.org,
	linux-gpio@vger.kernel.org, Sudip Mukherjee, Sascha Weisenberger
In-Reply-To: <CAHp75Ve3wA6AUTRA7RwFVBS=w5ntdwak+mTJN5tE_n7oW+JQvw@mail.gmail.com>

On 2017-05-18 12:11, Andy Shevchenko wrote:
> On Thu, May 18, 2017 at 8:20 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2017-05-13 15:36, Andy Shevchenko wrote:
>>> On Sat, May 13, 2017 at 10:29 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> First, the logic for translating a register bit to the return code of
>>>> exar_get_direction and exar_get_value were wrong. And second, there was
>>>> a flip regarding the register bank in exar_get_direction.
>>>
>>> Again, I wish it was tested in the first place.
>>>
>>> After addressing below:
>>> FWIW:
>>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>>>
>>>> @@ -68,7 +68,7 @@ static int exar_get(struct gpio_chip *chip, unsigned int reg)
>>>>         value = readb(exar_gpio->regs + reg);
>>>>         mutex_unlock(&exar_gpio->lock);
>>>>
>>>> -       return !!value;
>>>> +       return value;
>>>
>>> This one is correct.
> 
>>>
>>>> @@ -80,7 +80,7 @@ static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
>>>>         addr = bank ? EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
>>>>         val = exar_get(chip, addr) >> (offset % 8);
>>>>
>>>> -       return !!val;
>>>> +       return val & 1;
>>>
>>> It should be rather
>>>
>>>         val = exar_get(chip, addr) & BIT(offset % 8);
>>
>> That won't give us 0 or 1 as return value, thus would be incorrect.
> 
> Full picture:
> 
>  val = exar_get(chip, addr) & BIT(offset % 8);
> 
>  return !!val;
> 
> How it could be non-(1 or 0)?
> 

Right - but what is the point of that other style?

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

^ permalink raw reply

* Re: [PATCH 5/8] gpio: exar: Fix reading of directions and values
From: Andy Shevchenko @ 2017-05-18 10:11 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot,
	Linux Kernel Mailing List, linux-serial@vger.kernel.org,
	linux-gpio@vger.kernel.org, Sudip Mukherjee, Sascha Weisenberger
In-Reply-To: <88c38a4c-f275-28d3-694e-e3089e56062e@siemens.com>

On Thu, May 18, 2017 at 8:20 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2017-05-13 15:36, Andy Shevchenko wrote:
>> On Sat, May 13, 2017 at 10:29 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> First, the logic for translating a register bit to the return code of
>>> exar_get_direction and exar_get_value were wrong. And second, there was
>>> a flip regarding the register bank in exar_get_direction.
>>
>> Again, I wish it was tested in the first place.
>>
>> After addressing below:
>> FWIW:
>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>>
>>> @@ -68,7 +68,7 @@ static int exar_get(struct gpio_chip *chip, unsigned int reg)
>>>         value = readb(exar_gpio->regs + reg);
>>>         mutex_unlock(&exar_gpio->lock);
>>>
>>> -       return !!value;
>>> +       return value;
>>
>> This one is correct.

>>
>>> @@ -80,7 +80,7 @@ static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
>>>         addr = bank ? EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
>>>         val = exar_get(chip, addr) >> (offset % 8);
>>>
>>> -       return !!val;
>>> +       return val & 1;
>>
>> It should be rather
>>
>>         val = exar_get(chip, addr) & BIT(offset % 8);
>
> That won't give us 0 or 1 as return value, thus would be incorrect.

Full picture:

 val = exar_get(chip, addr) & BIT(offset % 8);

 return !!val;

How it could be non-(1 or 0)?

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH 5/8] gpio: exar: Fix reading of directions and values
From: Jan Kiszka @ 2017-05-18  5:20 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot,
	Linux Kernel Mailing List, linux-serial@vger.kernel.org,
	linux-gpio@vger.kernel.org, Sudip Mukherjee, Sascha Weisenberger
In-Reply-To: <CAHp75Vdxoro-FoJY-t+FVk_9J6OBZMhJ9P9qjhq5a2bMOJMW9A@mail.gmail.com>

On 2017-05-13 15:36, Andy Shevchenko wrote:
> On Sat, May 13, 2017 at 10:29 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> First, the logic for translating a register bit to the return code of
>> exar_get_direction and exar_get_value were wrong. And second, there was
>> a flip regarding the register bank in exar_get_direction.
> 
> Again, I wish it was tested in the first place.
> 
> After addressing below:
> FWIW:
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
>> @@ -68,7 +68,7 @@ static int exar_get(struct gpio_chip *chip, unsigned int reg)
>>         value = readb(exar_gpio->regs + reg);
>>         mutex_unlock(&exar_gpio->lock);
>>
>> -       return !!value;
>> +       return value;
> 
> This one is correct.
> 
>> @@ -80,7 +80,7 @@ static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
>>         addr = bank ? EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
>>         val = exar_get(chip, addr) >> (offset % 8);
>>
>> -       return !!val;
>> +       return val & 1;
> 
> It should be rather
> 
>         val = exar_get(chip, addr) & BIT(offset % 8);

That won't give us 0 or 1 as return value, thus would be incorrect.

> 
>>  }
>>
>>  static int exar_get_value(struct gpio_chip *chip, unsigned int offset)
>> @@ -89,10 +89,10 @@ static int exar_get_value(struct gpio_chip *chip, unsigned int offset)
>>         unsigned int addr;
>>         int val;
>>
>> -       addr = bank ? EXAR_OFFSET_MPIOLVL_LO : EXAR_OFFSET_MPIOLVL_HI;
>> +       addr = bank ? EXAR_OFFSET_MPIOLVL_HI : EXAR_OFFSET_MPIOLVL_LO;
> 
> Good catch!
> 
>>         val = exar_get(chip, addr) >> (offset % 8);
>>
>> -       return !!val;
>> +       return val & 1;
> 
> Ditto (see above).
> 

Same here.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

^ permalink raw reply

* Re: [PATCH 8/8] serial: exar: Add support for IOT2040 device
From: Jan Kiszka @ 2017-05-18  5:06 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot,
	Linux Kernel Mailing List, linux-serial@vger.kernel.org,
	linux-gpio@vger.kernel.org, Sudip Mukherjee, Sascha Weisenberger
In-Reply-To: <CAHp75VcG4rZiWoym9U4qaoEzUT_AY0m62oL+a8Zb-=3yBFV5Og@mail.gmail.com>

On 2017-05-13 15:54, Andy Shevchenko wrote:
> On Sat, May 13, 2017 at 10:29 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> This implements the setup of RS232 and the switch-over to RS485 or RS422
>> for the Siemens IOT2040. That uses an EXAR XR17V352 with external logic
>> to switch between the different modes. The external logic is controlled
>> via MPIO pins of the EXAR controller.
>>
>> Only pin 10 can be exported as GPIO on the IOT2040. It is connected to
>> an LED.
>>
>> As the XR17V352 used on the IOT2040 is not equipped with an external
>> EEPROM, it cannot present itself as IOT2040-variant via subvendor/
>> subdevice IDs. Thus, we have to check via DMI for the target platform.
>>
>> Co-developed with Sascha Weisenberger.
>>
> 
> Please, refactor that using properly formed DMI structrure and use its
> callback and driver data facilities/

Could you point to a specific example? The callback of the dmi_system_id
structure is not designed to handle device initializations like this one
- not to speak of having those two different initialization points here.

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

^ permalink raw reply

* Re: [PATCH V2 6/6] tty: serial: lpuart: add a more accurate baud rate calculation method
From: Stefan Agner @ 2017-05-17 17:35 UTC (permalink / raw)
  To: Dong Aisheng
  Cc: Dong Aisheng, linux-serial, linux-kernel, linux-arm-kernel,
	gregkh, jslaby, fugang.duan, Mingkai.Hu, yangbo.lu
In-Reply-To: <20170517034729.GD9913@b29396-OptiPlex-7040>

On 2017-05-16 20:47, Dong Aisheng wrote:
> On Mon, May 15, 2017 at 10:06:41AM -0700, Stefan Agner wrote:
>> On 2017-05-15 00:48, Dong Aisheng wrote:
>> > On new LPUART versions, the oversampling ratio for the receiver can be
>> > changed from 4x (00011) to 32x (11111) which could help us get a more
>> > accurate baud rate divider.
>> >
>> > The idea is to use the best OSR (over-sampling rate) possible.
>> > Note, OSR is typically hard-set to 16 in other LPUART instantiations.
>> > Loop to find the best OSR value possible, one that generates minimum
>> > baud diff iterate through the rest of the supported values of OSR.
>> >
>> > Currently only i.MX7ULP is using it.
>> >
>> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> > Cc: Jiri Slaby <jslaby@suse.com>
>> > Cc: Stefan Agner <stefan@agner.ch>
>> > Cc: Mingkai Hu <Mingkai.Hu@nxp.com>
>> > Cc: Yangbo Lu <yangbo.lu@nxp.com>
>> > Acked-by: Fugang Duan <fugang.duan@nxp.com>
>> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
>> > ---
>> >  drivers/tty/serial/fsl_lpuart.c | 85 ++++++++++++++++++++++++++++++++++++++---
>> >  1 file changed, 79 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
>> > index 107d0911..bda4b0c 100644
>> > --- a/drivers/tty/serial/fsl_lpuart.c
>> > +++ b/drivers/tty/serial/fsl_lpuart.c
>> > @@ -140,6 +140,8 @@
>> >  #define UARTBAUD_SBNS		0x00002000
>> >  #define UARTBAUD_SBR		0x00000000
>> >  #define UARTBAUD_SBR_MASK	0x1fff
>> > +#define UARTBAUD_OSR_MASK       0x1f
>> > +#define UARTBAUD_OSR_SHIFT      24
>> >
>> >  #define UARTSTAT_LBKDIF		0x80000000
>> >  #define UARTSTAT_RXEDGIF	0x40000000
>> > @@ -1506,6 +1508,72 @@ lpuart_set_termios(struct uart_port *port,
>> > struct ktermios *termios,
>> >  }
>> >
>> >  static void
>> > +lpuart32_serial_setbrg(struct lpuart_port *sport, unsigned int baudrate)
>> > +{
>> > +	u32 sbr, osr, baud_diff, tmp_osr, tmp_sbr, tmp_diff, tmp;
>> > +	u32 clk = sport->port.uartclk;
>> > +
>> > +	/*
>> > +	 * The idea is to use the best OSR (over-sampling rate) possible.
>> > +	 * Note, OSR is typically hard-set to 16 in other LPUART instantiations.
>> > +	 * Loop to find the best OSR value possible, one that generates minimum
>> > +	 * baud_diff iterate through the rest of the supported values of OSR.
>> > +	 *
>> > +	 * Calculation Formula:
>> > +	 *  Baud Rate = baud clock / ((OSR+1) × SBR)
>> > +	 */
>> > +	baud_diff = baudrate;
>> > +	osr = 0;
>> > +	sbr = 0;
>> > +
>> > +	for (tmp_osr = 4; tmp_osr <= 32; tmp_osr++) {
>> > +		/* calculate the temporary sbr value  */
>> > +		tmp_sbr = (clk / (baudrate * tmp_osr));
>> > +		if (tmp_sbr == 0)
>> > +			tmp_sbr = 1;
>> > +
>> > +		/*
>> > +		 * calculate the baud rate difference based on the temporary
>> > +		 * osr and sbr values
>> > +		 */
>> > +		tmp_diff = clk / (tmp_osr * tmp_sbr) - baudrate;
>> > +
>> > +		/* select best values between sbr and sbr+1 */
>> > +		tmp = clk / (tmp_osr * (tmp_sbr + 1));
>> > +		if (tmp_diff > (baudrate - tmp)) {
>> > +			tmp_diff = baudrate - tmp;
>> > +			tmp_sbr++;
>> > +		}
>> > +
>> > +		if (tmp_diff <= baud_diff) {
>> > +			baud_diff = tmp_diff;
>> > +			osr = tmp_osr;
>> > +			sbr = tmp_sbr;
>> > +		}
>> > +	}
>> > +
>> > +	/* handle buadrate outside acceptable rate */
>> > +	if (baud_diff > ((baudrate / 100) * 3))
>> > +		dev_warn(sport->port.dev,
>> > +			 "unacceptable baud rate difference of more than 3%%\n");
>> > +
>> > +	tmp = lpuart32_read(sport->port.membase + UARTBAUD);
>> > +
>> > +	if ((osr > 3) && (osr < 8))
>> > +		tmp |= UARTBAUD_BOTHEDGE;
>> > +
>> > +	tmp &= ~(UARTBAUD_OSR_MASK << UARTBAUD_OSR_SHIFT);
>> > +	tmp |= (((osr-1) & UARTBAUD_OSR_MASK) << UARTBAUD_OSR_SHIFT);
>> > +
>> > +	tmp &= ~UARTBAUD_SBR_MASK;
>> > +	tmp |= sbr & UARTBAUD_SBR_MASK;
>> > +
>> > +	tmp &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
>> > +
>> > +	lpuart32_write(tmp, sport->port.membase + UARTBAUD);
>> > +}
>> > +
>> > +static void
>> >  lpuart32_set_termios(struct uart_port *port, struct ktermios *termios,
>> >  		   struct ktermios *old)
>> >  {
>> > @@ -1611,12 +1679,17 @@ lpuart32_set_termios(struct uart_port *port,
>> > struct ktermios *termios,
>> >  	lpuart32_write(old_ctrl & ~(UARTCTRL_TE | UARTCTRL_RE),
>> >  			sport->port.membase + UARTCTRL);
>> >
>> > -	sbr = sport->port.uartclk / (16 * baud);
>> > -	bd &= ~UARTBAUD_SBR_MASK;
>> > -	bd |= sbr & UARTBAUD_SBR_MASK;
>> > -	bd |= UARTBAUD_BOTHEDGE;
>> > -	bd &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
>> > -	lpuart32_write(bd, sport->port.membase + UARTBAUD);
>> > +	if (of_device_is_compatible(port->dev->of_node, "fsl,imx7ulp-lpuart")) {
>>
>> Shouldn't we be consequent here and also use a flag in the soc data
>> instead of of_device_is_compatible...?
>>
> 
> The original purpose is that this is a temporary code and supposed will
> be deleted later once LS platforms confirmed the new baud setting API
> works for them as well.
> 
> That's why i did not make it a property, as i stated in the cover letter.

Ok, I see that is a good reason to not define a new feature property
now... 

But, if you are reasonable sure it should work, I am inclined to say
just enable it for LS1021a so it also really gets tested...

> 
>> Btw, instead of using 3 bools, I would prefer using a single flags like
>> your patchset is proposing for the GPIO driver, what do you think?
>>
> 
> Yes, good suggestion.
> Probably we could convert the below two.
> .is_32 = true,
> .is_be = true,
> 
> But reg_off seems better to be kept.

Sounds good!

--
Stefan

> 
> Regards
> Dong Aisheng
> 
>> --
>> Stefan
>>
>>
>> > +		lpuart32_serial_setbrg(sport, baud);
>> > +	} else {
>> > +		sbr = sport->port.uartclk / (16 * baud);
>> > +		bd &= ~UARTBAUD_SBR_MASK;
>> > +		bd |= sbr & UARTBAUD_SBR_MASK;
>> > +		bd |= UARTBAUD_BOTHEDGE;
>> > +		bd &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
>> > +		lpuart32_write(bd, sport->port.membase + UARTBAUD);
>> > +	}
>> > +
>> >  	lpuart32_write(modem, sport->port.membase + UARTMODIR);
>> >  	lpuart32_write(ctrl, sport->port.membase + UARTCTRL);
>> >  	/* restore control register */

^ permalink raw reply

* Re: [PATCH 0/4] serdev: fix broken lifetime assumptions
From: Rob Herring @ 2017-05-17 13:18 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20170517103958.GU3657@localhost>

On Wed, May 17, 2017 at 5:39 AM, Johan Hovold <johan@kernel.org> wrote:
> On Tue, Apr 11, 2017 at 07:07:27PM +0200, Johan Hovold wrote:
>> This series fixes a number of issues with the new serdev code, which was
>> based on incorrect tty-port lifetime assumptions.
>>
>> The first patch disables serdev support by reverting the patch which
>> hooked into the tty layer in a broken way that leads to crashes and
>> leaks when deregistering devices. This one should probably go into 4.11.
>>
>> The second patch fixes a specific bug in the tty-port client
>> registration code, while the third patch adds a new interface for
>> registering serdev devices. The final patch ultimately enables serdev
>> again for the serial drivers.
>>
>> More details can be found in the individual commit messages.
>>
>> Johan
>>
>>
>> Johan Hovold (4):
>>   Revert "tty_port: register tty ports with serdev bus"
>>   serdev: fix tty-port client deregistration
>>   tty/serdev: add serdev registration interface
>>   serial: enable serdev support
>
> Rob, did you have any further comments to this series?

No. For the series,

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

> Greg, I think these should go into 4.12-rc2, and the first patch should
> probably also have a stable tag now as serdev resource-management is
> clearly broken in 4.11.
>
> Johan

^ permalink raw reply

* Re: [PATCH] serdev: Restore serdev_device_write_buf for atomic context
From: Greg Kroah-Hartman @ 2017-05-17 12:46 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: Johan Hovold, Rob Herring, Jiri Slaby, Sebastian Reichel,
	Guenter Roeck, Andy Shevchenko, Andrey Smirnov,
	linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <8a24cad2-26d7-3394-509a-a0c038345d13@i2se.com>

On Wed, May 17, 2017 at 01:38:40PM +0200, Stefan Wahren wrote:
> Hi Greg,
> 
> Am 08.05.2017 um 17:18 schrieb Johan Hovold:
> > On Thu, May 04, 2017 at 03:32:53PM -0500, Rob Herring wrote:
> >> On Thu, May 4, 2017 at 11:22 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> >>> Am 02.05.2017 um 15:18 schrieb Johan Hovold:
> >>>> On Tue, May 02, 2017 at 07:41:34AM -0500, Rob Herring wrote:
> >>>>> On Tue, May 2, 2017 at 4:06 AM, Johan Hovold <johan@kernel.org> wrote:
> >>>>>> On Fri, Apr 28, 2017 at 01:47:21PM +0200, Stefan Wahren wrote:
> >>>>>>> Starting with commit 6fe729c4bdae ("serdev: Add serdev_device_write
> >>>>>>> subroutine") the function serdev_device_write_buf cannot be used in
> >>>>>>> atomic context anymore (mutex_lock is sleeping). So restore the old
> >>>>>>> behavior.
> >>>>>> Yeah, preventing use in atomic context seems unnecessary, although any
> >>>>>> clients writing must now deal with serialisation themselves (as before,
> >>>>>> and as they should).
> >>>>> We could just remove the mutex for serdev_device_write and always make
> >>>>> the client responsible for serialization.
> >>>> That sounds reasonable.
> >>> So it's unwanted to have 2 write functions (non-atomic, atomic)?
> >> No, it's unwanted to have more than we need.
> >>
> >> Looking closer, we'd also have to ensure the wait for completion is
> >> not called also. So probably better to just leave it as you have done
> >> it.
> > Indeed. Sorry if my reply above was unclear on that point (i.e. that
> > Stefan's patch is still needed regardless of whether we keep the mutex
> > or not).
> >
> > Thanks,
> > Johan
> 
> are you okay with this patch and can you please apply it?

I'll work to catch up on tty/serial patches soon...

^ permalink raw reply

* Re: [PATCH v5 16/17] tty: serdev-ttyport: return actual baudrate from ttyport_set_baudrate
From: Stefan Wahren @ 2017-05-17 11:58 UTC (permalink / raw)
  To: Rob Herring, David S. Miller, Greg Kroah-Hartman
  Cc: Mark Rutland, Jiri Slaby, linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Lino Sanfilippo, Jakub Kicinski
In-Reply-To: <1494406408-31760-17-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org>

Hi Greg,

Am 10.05.2017 um 10:53 schrieb Stefan Wahren:
> Instead of returning the requested baudrate, we better return the
> actual one because it isn't always the same.
>
> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>  drivers/tty/serdev/serdev-ttyport.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
> index 487c88f..2cfdf34 100644
> --- a/drivers/tty/serdev/serdev-ttyport.c
> +++ b/drivers/tty/serdev/serdev-ttyport.c
> @@ -151,7 +151,7 @@ static unsigned int ttyport_set_baudrate(struct serdev_controller *ctrl, unsigne
>  
>  	/* tty_set_termios() return not checked as it is always 0 */
>  	tty_set_termios(tty, &ktermios);
> -	return speed;
> +	return ktermios.c_ospeed;
>  }
>  
>  static void ttyport_set_flow_control(struct serdev_controller *ctrl, bool enable)

there is no dependency to this series. I would be happy if this could be
applied, so i don't have to carry it for the next version.

Thanks
Stefan
--
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


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