[PATCH v3 00/10] serial/gpio: exar: Fixes and support for IOT2000

[PATCH v3 00/10] serial/gpio: exar: Fixes and support for IOT2000

[Jan Kiszka]

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 v3:
 - fix MPIO state for Commtech adapters (regression of merged patch from
   previous round)
 - do not create gpio device for Commtech adapters
 - switch back to device properties
 - pass parent reference via device.parent instead of platform data
 - use dmi_system_id table instead of open-coded matching
 - address some smaller review remarks
 - fix reading back of rs485 state
 - adjust parenthood of exar gpiochip

Jan

Jan Kiszka (10):
  serial: exar: Leave MPIOs as output for Commtech adapters
  gpio-exar/8250-exar: Do not even instantiate a GPIO device for
    Commtech cards
  gpio-exar/8250-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: Rearrange gpiochip parenthood
  gpio: exar: Refactor address and bit calculations
  gpio-exar/8250-exar: Make set of exported GPIOs configurable
  serial: exar: Add support for IOT2040 device

 drivers/gpio/gpio-exar.c            |  74 ++++++++-------
 drivers/tty/serial/8250/8250_exar.c | 180 ++++++++++++++++++++++++++++++++++--
 2 files changed, 207 insertions(+), 47 deletions(-)

-- 
2.12.0

[PATCH v3 01/10] serial: exar: Leave MPIOs as output for Commtech adapters

[Jan Kiszka]

Commtech adapters apparently need the original setting as outputs, see
https://marc.info/?l=linux-gpio&m=149557425201323&w=2. Account for that.

Fixes: 7dea8165f1d6 ("serial: exar: Preconfigure xr17v35x MPIOs as output")

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/tty/serial/8250/8250_exar.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index b4fa585156c7..8984e8b2d524 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -171,19 +171,26 @@ pci_xr17c154_setup(struct exar8250 *priv, struct pci_dev *pcidev,
 	return default_setup(priv, pcidev, idx, offset, port);
 }
 
-static void setup_gpio(u8 __iomem *p)
+static void setup_gpio(struct pci_dev *pcidev, u8 __iomem *p)
 {
+	/*
+	 * The Commtech adapters required the MPIOs to be driven low. The Exar
+	 * devices will export them as GPIOs, so we pre-configure them safely
+	 * as inputs.
+	 */
+	u8 dir = pcidev->vendor == PCI_VENDOR_ID_EXAR ? 0xff : 0x00;
+
 	writeb(0x00, p + UART_EXAR_MPIOINT_7_0);
 	writeb(0x00, p + UART_EXAR_MPIOLVL_7_0);
 	writeb(0x00, p + UART_EXAR_MPIO3T_7_0);
 	writeb(0x00, p + UART_EXAR_MPIOINV_7_0);
-	writeb(0xff, p + UART_EXAR_MPIOSEL_7_0);
+	writeb(dir,  p + UART_EXAR_MPIOSEL_7_0);
 	writeb(0x00, p + UART_EXAR_MPIOOD_7_0);
 	writeb(0x00, p + UART_EXAR_MPIOINT_15_8);
 	writeb(0x00, p + UART_EXAR_MPIOLVL_15_8);
 	writeb(0x00, p + UART_EXAR_MPIO3T_15_8);
 	writeb(0x00, p + UART_EXAR_MPIOINV_15_8);
-	writeb(0xff, p + UART_EXAR_MPIOSEL_15_8);
+	writeb(dir,  p + UART_EXAR_MPIOSEL_15_8);
 	writeb(0x00, p + UART_EXAR_MPIOOD_15_8);
 }
 
@@ -236,7 +243,7 @@ pci_xr17v35x_setup(struct exar8250 *priv, struct pci_dev *pcidev,
 
 	if (idx == 0) {
 		/* Setup Multipurpose Input/Output pins. */
-		setup_gpio(p);
+		setup_gpio(pcidev, p);
 
 		port->port.private_data = xr17v35x_register_gpio(pcidev);
 	}
-- 
2.12.0

[PATCH v3 02/10] gpio-exar/8250-exar: Do not even instantiate a GPIO device for Commtech cards

[Jan Kiszka]

Commtech adapters need the MPIOs for internal purposes, and the
gpio-exar driver already refused to pick them up. But there is actually
no point in even creating the underlying platform device.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/gpio/gpio-exar.c            | 3 ---
 drivers/tty/serial/8250/8250_exar.c | 4 +++-
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
index 081076771217..006a9a67c2a4 100644
--- a/drivers/gpio/gpio-exar.c
+++ b/drivers/gpio/gpio-exar.c
@@ -124,9 +124,6 @@ static int gpio_exar_probe(struct platform_device *pdev)
 	void __iomem *p;
 	int index, ret;
 
-	if (pcidev->vendor != PCI_VENDOR_ID_EXAR)
-		return -ENODEV;
-
 	/*
 	 * Map the pci device to get the register addresses.
 	 * We will need to read and write those registers to control
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index 8984e8b2d524..c29c7e675890 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -245,7 +245,9 @@ pci_xr17v35x_setup(struct exar8250 *priv, struct pci_dev *pcidev,
 		/* Setup Multipurpose Input/Output pins. */
 		setup_gpio(pcidev, p);
 
-		port->port.private_data = xr17v35x_register_gpio(pcidev);
+		if (pcidev->vendor == PCI_VENDOR_ID_EXAR)
+			port->port.private_data =
+				xr17v35x_register_gpio(pcidev);
 	}
 
 	return 0;
-- 
2.12.0

[PATCH v3 03/10] gpio-exar/8250-exar: Fix passing in of parent PCI device

[Jan Kiszka]

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.

Set the platform device parent instead.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/gpio/gpio-exar.c            | 2 +-
 drivers/tty/serial/8250/8250_exar.c | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
index 006a9a67c2a4..da4550bb9939 100644
--- a/drivers/gpio/gpio-exar.c
+++ b/drivers/gpio/gpio-exar.c
@@ -119,7 +119,7 @@ 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 pci_dev *pcidev = to_pci_dev(pdev->dev.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 c29c7e675890..0f4b236d7e68 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -203,7 +203,8 @@ xr17v35x_register_gpio(struct pci_dev *pcidev)
 	if (!pdev)
 		return NULL;
 
-	platform_set_drvdata(pdev, pcidev);
+	pdev->dev.parent = &pcidev->dev;
+
 	if (platform_device_add(pdev) < 0) {
 		platform_device_put(pdev);
 		return NULL;
-- 
2.12.0

[PATCH v3 04/10] gpio: exar: Allocate resources on behalf of the platform device

[Jan Kiszka]

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 da4550bb9939..65126fa1e512 100644
--- a/drivers/gpio/gpio-exar.c
+++ b/drivers/gpio/gpio-exar.c
@@ -136,7 +136,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;
 
@@ -157,7 +157,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

[PATCH v3 05/10] gpio: exar: Fix iomap request

[Jan Kiszka]

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 65126fa1e512..b29890b143ce 100644
--- a/drivers/gpio/gpio-exar.c
+++ b/drivers/gpio/gpio-exar.c
@@ -125,14 +125,10 @@ static int gpio_exar_probe(struct platform_device *pdev)
 	int index, ret;
 
 	/*
-	 * 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

[PATCH v3 06/10] gpio: exar: Fix reading of directions and values

[Jan Kiszka]

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>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.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 b29890b143ce..f3585a184f39 100644
--- a/drivers/gpio/gpio-exar.c
+++ b/drivers/gpio/gpio-exar.c
@@ -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;
 }
 
 static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
@@ -78,7 +78,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;
 }
@@ -89,8 +89,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

[PATCH v3 07/10] gpio-exar/8250-exar: Rearrange gpiochip parenthood

[Jan Kiszka]

Set the parent of the exar gpiochip to its platform device, like other
gpiochips are doing it. In order to keep the relationship discoverable
for ACPI systems, set the platform device companion to the PCI device.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/gpio/gpio-exar.c            | 2 +-
 drivers/tty/serial/8250/8250_exar.c | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
index f3585a184f39..1a629831d45b 100644
--- a/drivers/gpio/gpio-exar.c
+++ b/drivers/gpio/gpio-exar.c
@@ -142,7 +142,7 @@ static int gpio_exar_probe(struct platform_device *pdev)
 
 	sprintf(exar_gpio->name, "exar_gpio%d", index);
 	exar_gpio->gpio_chip.label = exar_gpio->name;
-	exar_gpio->gpio_chip.parent = &pcidev->dev;
+	exar_gpio->gpio_chip.parent = &pdev->dev;
 	exar_gpio->gpio_chip.direction_output = exar_direction_output;
 	exar_gpio->gpio_chip.direction_input = exar_direction_input;
 	exar_gpio->gpio_chip.get_direction = exar_get_direction;
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index 0f4b236d7e68..ee4b142f3ed0 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -9,6 +9,7 @@
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation; either version 2 of the License.
  */
+#include <linux/acpi.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -204,6 +205,7 @@ xr17v35x_register_gpio(struct pci_dev *pcidev)
 		return NULL;
 
 	pdev->dev.parent = &pcidev->dev;
+	ACPI_COMPANION_SET(&pdev->dev, ACPI_COMPANION(&pcidev->dev));
 
 	if (platform_device_add(pdev) < 0) {
 		platform_device_put(pdev);
-- 
2.12.0

[PATCH v3 08/10] gpio: exar: Refactor address and bit calculations

[Jan Kiszka]

Make the address and bit calculation more friendly for introducing a
"first pin" offset later on. The new form is also more compact and
regular.

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

diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
index 1a629831d45b..d13bf20b8639 100644
--- a/drivers/gpio/gpio-exar.c
+++ b/drivers/gpio/gpio-exar.c
@@ -51,11 +51,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;
+	unsigned int addr = offset / 8 ?
+		EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
+	unsigned int bit  = offset % 8;
 
-	addr = bank ? EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
-	exar_update(chip, addr, direction, offset % 8);
+	exar_update(chip, addr, direction, bit);
 	return 0;
 }
 
@@ -73,36 +73,30 @@ 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;
-	int val;
+	unsigned int addr = offset / 8 ?
+		EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
+	unsigned int bit  = offset % 8;
 
-	addr = bank ? EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
-	val = exar_get(chip, addr) & BIT(offset % 8);
-
-	return !!val;
+	return !!(exar_get(chip, addr) & BIT(bit));
 }
 
 static int exar_get_value(struct gpio_chip *chip, unsigned int offset)
 {
-	unsigned int bank = offset / 8;
-	unsigned int addr;
-	int val;
-
-	addr = bank ? EXAR_OFFSET_MPIOLVL_HI : EXAR_OFFSET_MPIOLVL_LO;
-	val = exar_get(chip, addr) & BIT(offset % 8);
+	unsigned int addr = offset / 8 ?
+		EXAR_OFFSET_MPIOLVL_HI : EXAR_OFFSET_MPIOLVL_LO;
+	unsigned int bit  = offset % 8;
 
-	return !!val;
+	return !!(exar_get(chip, addr) & BIT(bit));
 }
 
 static void exar_set_value(struct gpio_chip *chip, unsigned int offset,
 			   int value)
 {
-	unsigned int bank = offset / 8;
-	unsigned int addr;
+	unsigned int addr = offset / 8 ?
+		EXAR_OFFSET_MPIOLVL_HI : EXAR_OFFSET_MPIOLVL_LO;
+	unsigned int bit  = offset % 8;
 
-	addr = bank ? EXAR_OFFSET_MPIOLVL_HI : EXAR_OFFSET_MPIOLVL_LO;
-	exar_update(chip, addr, value, offset % 8);
+	exar_update(chip, addr, value, bit);
 }
 
 static int exar_direction_output(struct gpio_chip *chip, unsigned int offset,
-- 
2.12.0

[PATCH v3 09/10] gpio-exar/8250-exar: Make set of exported GPIOs configurable

[Jan Kiszka]

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            | 29 ++++++++++++++++++++---------
 drivers/tty/serial/8250/8250_exar.c | 14 +++++++++++---
 2 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
index d13bf20b8639..8970f50e681e 100644
--- a/drivers/gpio/gpio-exar.c
+++ b/drivers/gpio/gpio-exar.c
@@ -31,6 +31,7 @@ struct exar_gpio_chip {
 	int index;
 	void __iomem *regs;
 	char name[20];
+	unsigned int first_pin;
 };
 
 static void exar_update(struct gpio_chip *chip, unsigned int reg, int val,
@@ -51,9 +52,10 @@ 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 addr = offset / 8 ?
+	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
+	unsigned int addr = (offset + exar_gpio->first_pin) / 8 ?
 		EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
-	unsigned int bit  = offset % 8;
+	unsigned int bit  = (offset + exar_gpio->first_pin) % 8;
 
 	exar_update(chip, addr, direction, bit);
 	return 0;
@@ -73,18 +75,20 @@ 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 addr = offset / 8 ?
+	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
+	unsigned int addr = (offset + exar_gpio->first_pin) / 8 ?
 		EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
-	unsigned int bit  = offset % 8;
+	unsigned int bit  = (offset + exar_gpio->first_pin) % 8;
 
 	return !!(exar_get(chip, addr) & BIT(bit));
 }
 
 static int exar_get_value(struct gpio_chip *chip, unsigned int offset)
 {
-	unsigned int addr = offset / 8 ?
+	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
+	unsigned int addr = (offset + exar_gpio->first_pin) / 8 ?
 		EXAR_OFFSET_MPIOLVL_HI : EXAR_OFFSET_MPIOLVL_LO;
-	unsigned int bit  = offset % 8;
+	unsigned int bit  = (offset + exar_gpio->first_pin) % 8;
 
 	return !!(exar_get(chip, addr) & BIT(bit));
 }
@@ -92,9 +96,10 @@ 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 addr = offset / 8 ?
+	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
+	unsigned int addr = (offset + exar_gpio->first_pin) / 8 ?
 		EXAR_OFFSET_MPIOLVL_HI : EXAR_OFFSET_MPIOLVL_LO;
-	unsigned int bit  = offset % 8;
+	unsigned int bit  = (offset + exar_gpio->first_pin) % 8;
 
 	exar_update(chip, addr, value, bit);
 }
@@ -115,6 +120,7 @@ static int gpio_exar_probe(struct platform_device *pdev)
 {
 	struct pci_dev *pcidev = to_pci_dev(pdev->dev.parent);
 	struct exar_gpio_chip *exar_gpio;
+	u32 first_pin, npins;
 	void __iomem *p;
 	int index, ret;
 
@@ -126,6 +132,10 @@ static int gpio_exar_probe(struct platform_device *pdev)
 	if (!p)
 		return -ENOMEM;
 
+	if (device_property_read_u32(&pdev->dev, "first_pin", &first_pin) < 0 ||
+	    device_property_read_u32(&pdev->dev, "npins", &npins) < 0)
+		return -EINVAL;
+
 	exar_gpio = devm_kzalloc(&pdev->dev, sizeof(*exar_gpio), GFP_KERNEL);
 	if (!exar_gpio)
 		return -ENOMEM;
@@ -143,9 +153,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 = npins;
 	exar_gpio->regs = p;
 	exar_gpio->index = index;
+	exar_gpio->first_pin = first_pin;
 
 	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 ee4b142f3ed0..42e7c5149c07 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -14,6 +14,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/pci.h>
+#include <linux/property.h>
 #include <linux/serial_core.h>
 #include <linux/serial_reg.h>
 #include <linux/slab.h>
@@ -196,8 +197,14 @@ static void setup_gpio(struct pci_dev *pcidev, u8 __iomem *p)
 }
 
 static void *
-xr17v35x_register_gpio(struct pci_dev *pcidev)
+xr17v35x_register_gpio(struct pci_dev *pcidev, unsigned int first_pin,
+		       unsigned int npins)
 {
+	struct property_entry properties[] = {
+		PROPERTY_ENTRY_U32("first_pin", first_pin),
+		PROPERTY_ENTRY_U32("npins", npins),
+		{ }
+	};
 	struct platform_device *pdev;
 
 	pdev = platform_device_alloc("gpio_exar", PLATFORM_DEVID_AUTO);
@@ -207,7 +214,8 @@ xr17v35x_register_gpio(struct pci_dev *pcidev)
 	pdev->dev.parent = &pcidev->dev;
 	ACPI_COMPANION_SET(&pdev->dev, ACPI_COMPANION(&pcidev->dev));
 
-	if (platform_device_add(pdev) < 0) {
+	if (platform_device_add_properties(pdev, properties) < 0 ||
+	    platform_device_add(pdev) < 0) {
 		platform_device_put(pdev);
 		return NULL;
 	}
@@ -250,7 +258,7 @@ pci_xr17v35x_setup(struct exar8250 *priv, struct pci_dev *pcidev,
 
 		if (pcidev->vendor == PCI_VENDOR_ID_EXAR)
 			port->port.private_data =
-				xr17v35x_register_gpio(pcidev);
+				xr17v35x_register_gpio(pcidev, 0, 16);
 	}
 
 	return 0;
-- 
2.12.0

[PATCH v3 10/10] serial: exar: Add support for IOT2040 device

[Jan Kiszka]

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.

Signed-off-by: Sascha Weisenberger <sascha.weisenberger@siemens.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/tty/serial/8250/8250_exar.c | 154 ++++++++++++++++++++++++++++++++++--
 1 file changed, 148 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index 42e7c5149c07..625284f96aba 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -10,6 +10,7 @@
  * the Free Software Foundation; either version 2 of the License.
  */
 #include <linux/acpi.h>
+#include <linux/dmi.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -62,8 +63,50 @@
 #define UART_EXAR_MPIOSEL_15_8	0x99	/* MPIOSEL[15:8] */
 #define UART_EXAR_MPIOOD_15_8	0x9a	/* MPIOOD[15:8] */
 
+#define UART_EXAR_RS485_DLY(x)	((x) << 4)
+
+/*
+ * IOT2040 MPIO wiring semantics:
+ *
+ * MPIO		Port	Function
+ * ----		----	--------
+ * 0		2 	Mode bit 0
+ * 1		2	Mode bit 1
+ * 2		2	Terminate bus
+ * 3		-	<reserved>
+ * 4		3	Mode bit 0
+ * 5		3	Mode bit 1
+ * 6		3	Terminate bus
+ * 7		-	<reserved>
+ * 8		2	Enable
+ * 9		3	Enable
+ * 10		-	Red LED
+ * 11..15	-	<unused>
+ */
+
+/* IOT2040 MPIOs 0..7 */
+#define IOT2040_UART_MODE_RS232		0x01
+#define IOT2040_UART_MODE_RS485		0x02
+#define IOT2040_UART_MODE_RS422		0x03
+#define IOT2040_UART_TERMINATE_BUS	0x04
+
+#define IOT2040_UART1_MASK		0x0f
+#define IOT2040_UART2_SHIFT		4
+
+#define IOT2040_UARTS_DEFAULT_MODE	0x11	/* both RS232 */
+#define IOT2040_UARTS_GPIO_LO_MODE	0x88	/* reserved pins as input */
+
+/* IOT2040 MPIOs 8..15 */
+#define IOT2040_UARTS_ENABLE		0x03
+#define IOT2040_UARTS_GPIO_HI_MODE	0xF8	/* enable & LED as outputs */
+
 struct exar8250;
 
+struct exar8250_platform {
+	int (*rs485_config)(struct uart_port *, struct serial_rs485 *);
+	int (*register_gpio)(struct pci_dev *, struct uart_8250_port *);
+};
+
 /**
  * struct exar8250_board - board information
  * @num_ports: number of serial ports
@@ -197,8 +240,8 @@ static void setup_gpio(struct pci_dev *pcidev, u8 __iomem *p)
 }
 
 static void *
-xr17v35x_register_gpio(struct pci_dev *pcidev, unsigned int first_pin,
-		       unsigned int npins)
+__xr17v35x_register_gpio(struct pci_dev *pcidev, unsigned int first_pin,
+			 unsigned int npins)
 {
 	struct property_entry properties[] = {
 		PROPERTY_ENTRY_U32("first_pin", first_pin),
@@ -223,17 +266,118 @@ xr17v35x_register_gpio(struct pci_dev *pcidev, unsigned int first_pin,
 	return pdev;
 }
 
+static int xr17v35x_register_gpio(struct pci_dev *pcidev,
+				  struct uart_8250_port *port)
+{
+	if (pcidev->vendor == PCI_VENDOR_ID_EXAR)
+		port->port.private_data =
+			__xr17v35x_register_gpio(pcidev, 0, 16);
+
+	return 0;
+}
+
+static int iot2040_rs485_config(struct uart_port *port,
+				struct serial_rs485 *rs485)
+{
+	bool is_rs485 = !!(rs485->flags & SER_RS485_ENABLED);
+	u8 __iomem *p = port->membase;
+	u8 mask = IOT2040_UART1_MASK;
+	u8 mode, value;
+
+	if (is_rs485) {
+		if (rs485->flags & SER_RS485_RX_DURING_TX)
+			mode = IOT2040_UART_MODE_RS422;
+		else
+			mode = IOT2040_UART_MODE_RS485;
+
+		if (rs485->flags & SER_RS485_TERMINATE_BUS)
+			mode |= IOT2040_UART_TERMINATE_BUS;
+	} else {
+		mode = IOT2040_UART_MODE_RS232;
+	}
+
+	if (port->line == 3) {
+		mask <<= IOT2040_UART2_SHIFT;
+		mode <<= IOT2040_UART2_SHIFT;
+	}
+
+	value = readb(p + UART_EXAR_MPIOLVL_7_0);
+	value &= ~mask;
+	value |= mode;
+	writeb(value, p + UART_EXAR_MPIOLVL_7_0);
+
+	value = readb(p + UART_EXAR_FCTR);
+	if (is_rs485)
+		value |= UART_FCTR_EXAR_485;
+	else
+		value &= ~UART_FCTR_EXAR_485;
+	writeb(value, p + UART_EXAR_FCTR);
+
+	if (is_rs485)
+		writeb(UART_EXAR_RS485_DLY(4), p + UART_MSR);
+
+	port->rs485 = *rs485;
+
+	return 0;
+}
+
+static int iot2040_register_gpio(struct pci_dev *pcidev,
+			      struct uart_8250_port *port)
+{
+	u8 __iomem *p = port->port.membase;
+
+	writeb(IOT2040_UARTS_DEFAULT_MODE, p + UART_EXAR_MPIOLVL_7_0);
+	writeb(IOT2040_UARTS_GPIO_LO_MODE, p + UART_EXAR_MPIOSEL_7_0);
+	writeb(IOT2040_UARTS_ENABLE, p + UART_EXAR_MPIOLVL_15_8);
+	writeb(IOT2040_UARTS_GPIO_HI_MODE, p + UART_EXAR_MPIOSEL_15_8);
+
+	port->port.private_data = __xr17v35x_register_gpio(pcidev, 10, 1);
+
+	return 0;
+}
+
+static const struct exar8250_platform exar8250_default_platform = {
+	.register_gpio = xr17v35x_register_gpio,
+};
+
+static const struct exar8250_platform iot2040_platform = {
+	.rs485_config = iot2040_rs485_config,
+	.register_gpio = iot2040_register_gpio,
+};
+
+static const struct dmi_system_id exar_platforms[] = {
+	{
+		.matches = {
+			DMI_EXACT_MATCH(DMI_BOARD_NAME, "SIMATIC IOT2000"),
+			DMI_EXACT_MATCH(DMI_BOARD_ASSET_TAG,
+					"6ES7647-0AA00-1YA2"),
+		},
+		.driver_data = (void *)&iot2040_platform,
+	},
+	{}
+};
+
 static int
 pci_xr17v35x_setup(struct exar8250 *priv, struct pci_dev *pcidev,
 		   struct uart_8250_port *port, int idx)
 {
 	const struct exar8250_board *board = priv->board;
+	const struct exar8250_platform *platform;
+	const struct dmi_system_id *dmi_match;
 	unsigned int offset = idx * 0x400;
 	unsigned int baud = 7812500;
 	u8 __iomem *p;
 	int ret;
 
+	dmi_match = dmi_first_match(exar_platforms);
+	if (dmi_match)
+		platform = dmi_match->driver_data;
+	else
+		platform = &exar8250_default_platform;
+
 	port->port.uartclk = baud * 16;
+	port->port.rs485_config = platform->rs485_config;
+
 	/*
 	 * Setup the uart clock for the devices on expansion slot to
 	 * half the clock speed of the main chip (which is 125MHz)
@@ -256,12 +400,10 @@ pci_xr17v35x_setup(struct exar8250 *priv, struct pci_dev *pcidev,
 		/* Setup Multipurpose Input/Output pins. */
 		setup_gpio(pcidev, p);
 
-		if (pcidev->vendor == PCI_VENDOR_ID_EXAR)
-			port->port.private_data =
-				xr17v35x_register_gpio(pcidev, 0, 16);
+		ret = platform->register_gpio(pcidev, port);
 	}
 
-	return 0;
+	return ret;
 }
 
 static void pci_xr17v35x_exit(struct pci_dev *pcidev)
-- 
2.12.0

Re: [PATCH v3 02/10] gpio-exar/8250-exar: Do not even instantiate a GPIO device for Commtech cards

[Andy Shevchenko]

On Fri, May 26, 2017 at 7:02 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> Commtech adapters need the MPIOs for internal purposes, and the
> gpio-exar driver already refused to pick them up. But there is actually
> no point in even creating the underlying platform device.

FWIW:
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  drivers/gpio/gpio-exar.c            | 3 ---
>  drivers/tty/serial/8250/8250_exar.c | 4 +++-
>  2 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
> index 081076771217..006a9a67c2a4 100644
> --- a/drivers/gpio/gpio-exar.c
> +++ b/drivers/gpio/gpio-exar.c
> @@ -124,9 +124,6 @@ static int gpio_exar_probe(struct platform_device *pdev)
>         void __iomem *p;
>         int index, ret;
>
> -       if (pcidev->vendor != PCI_VENDOR_ID_EXAR)
> -               return -ENODEV;
> -
>         /*
>          * Map the pci device to get the register addresses.
>          * We will need to read and write those registers to control
> diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
> index 8984e8b2d524..c29c7e675890 100644
> --- a/drivers/tty/serial/8250/8250_exar.c
> +++ b/drivers/tty/serial/8250/8250_exar.c
> @@ -245,7 +245,9 @@ pci_xr17v35x_setup(struct exar8250 *priv, struct pci_dev *pcidev,
>                 /* Setup Multipurpose Input/Output pins. */
>                 setup_gpio(pcidev, p);
>
> -               port->port.private_data = xr17v35x_register_gpio(pcidev);
> +               if (pcidev->vendor == PCI_VENDOR_ID_EXAR)
> +                       port->port.private_data =
> +                               xr17v35x_register_gpio(pcidev);
>         }
>
>         return 0;
> --
> 2.12.0
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 03/10] gpio-exar/8250-exar: Fix passing in of parent PCI device

[Andy Shevchenko]

On Fri, May 26, 2017 at 7:02 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> 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.
>
> Set the platform device parent instead.

Now this one looks much better than first efforts.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  drivers/gpio/gpio-exar.c            | 2 +-
>  drivers/tty/serial/8250/8250_exar.c | 3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
> index 006a9a67c2a4..da4550bb9939 100644
> --- a/drivers/gpio/gpio-exar.c
> +++ b/drivers/gpio/gpio-exar.c
> @@ -119,7 +119,7 @@ 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 pci_dev *pcidev = to_pci_dev(pdev->dev.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 c29c7e675890..0f4b236d7e68 100644
> --- a/drivers/tty/serial/8250/8250_exar.c
> +++ b/drivers/tty/serial/8250/8250_exar.c
> @@ -203,7 +203,8 @@ xr17v35x_register_gpio(struct pci_dev *pcidev)
>         if (!pdev)
>                 return NULL;
>
> -       platform_set_drvdata(pdev, pcidev);
> +       pdev->dev.parent = &pcidev->dev;
> +
>         if (platform_device_add(pdev) < 0) {
>                 platform_device_put(pdev);
>                 return NULL;
> --
> 2.12.0
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 07/10] gpio-exar/8250-exar: Rearrange gpiochip parenthood

[Andy Shevchenko]

On Fri, May 26, 2017 at 7:02 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> Set the parent of the exar gpiochip to its platform device, like other
> gpiochips are doing it. In order to keep the relationship discoverable
> for ACPI systems, set the platform device companion to the PCI device.
>

Setting companion is a right thing to do (like it's done in other similar cases)

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  drivers/gpio/gpio-exar.c            | 2 +-
>  drivers/tty/serial/8250/8250_exar.c | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
> index f3585a184f39..1a629831d45b 100644
> --- a/drivers/gpio/gpio-exar.c
> +++ b/drivers/gpio/gpio-exar.c
> @@ -142,7 +142,7 @@ static int gpio_exar_probe(struct platform_device *pdev)
>
>         sprintf(exar_gpio->name, "exar_gpio%d", index);
>         exar_gpio->gpio_chip.label = exar_gpio->name;
> -       exar_gpio->gpio_chip.parent = &pcidev->dev;
> +       exar_gpio->gpio_chip.parent = &pdev->dev;
>         exar_gpio->gpio_chip.direction_output = exar_direction_output;
>         exar_gpio->gpio_chip.direction_input = exar_direction_input;
>         exar_gpio->gpio_chip.get_direction = exar_get_direction;
> diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
> index 0f4b236d7e68..ee4b142f3ed0 100644
> --- a/drivers/tty/serial/8250/8250_exar.c
> +++ b/drivers/tty/serial/8250/8250_exar.c
> @@ -9,6 +9,7 @@
>   * it under the terms of the GNU General Public License as published by
>   * the Free Software Foundation; either version 2 of the License.
>   */
> +#include <linux/acpi.h>
>  #include <linux/io.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> @@ -204,6 +205,7 @@ xr17v35x_register_gpio(struct pci_dev *pcidev)
>                 return NULL;
>
>         pdev->dev.parent = &pcidev->dev;
> +       ACPI_COMPANION_SET(&pdev->dev, ACPI_COMPANION(&pcidev->dev));
>
>         if (platform_device_add(pdev) < 0) {
>                 platform_device_put(pdev);
> --
> 2.12.0
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 08/10] gpio: exar: Refactor address and bit calculations

[Andy Shevchenko]

On Fri, May 26, 2017 at 7:02 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> Make the address and bit calculation more friendly for introducing a
> "first pin" offset later on. The new form is also more compact and
> regular.

I think this should be folded to patch 9 since the latter touches near
all lines you touched here.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 09/10] gpio-exar/8250-exar: Make set of exported GPIOs configurable

[Andy Shevchenko]

On Fri, May 26, 2017 at 7:02 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> 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.

> +       unsigned int first_pin;

> +       if (device_property_read_u32(&pdev->dev, "first_pin", &first_pin) < 0 ||

And again, we need to follow the rules of the device property
bindings. (No underscores - use dashes; name should be registered).

Without Rafael's / Mika's and Rob's opinions I would not go with a
such name even for internal property.

DWC3 (USB) for example is still using "linux," prefix. I dunno if it's
the case here, I mean can or can't we use similar approach.

> +           device_property_read_u32(&pdev->dev, "npins", &npins) < 0)

"ngpios"

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 10/10] serial: exar: Add support for IOT2040 device

[Andy Shevchenko]

On Fri, May 26, 2017 at 7:02 PM, 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.

>  static void *
> -xr17v35x_register_gpio(struct pci_dev *pcidev, unsigned int first_pin,
> -                      unsigned int npins)
> +__xr17v35x_register_gpio(struct pci_dev *pcidev, unsigned int first_pin,
> +                        unsigned int npins)

> +static int xr17v35x_register_gpio(struct pci_dev *pcidev,
> +                                 struct uart_8250_port *port)
> +{
> +       if (pcidev->vendor == PCI_VENDOR_ID_EXAR)
> +               port->port.private_data =
> +                       __xr17v35x_register_gpio(pcidev, 0, 16);
> +
> +       return 0;
> +}

> -               if (pcidev->vendor == PCI_VENDOR_ID_EXAR)
> -                       port->port.private_data =
> -                               xr17v35x_register_gpio(pcidev, 0, 16);
> +               ret = platform->register_gpio(pcidev, port);
>         }

Can't you move at least some of this to the patch where you move the check?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 09/10] gpio-exar/8250-exar: Make set of exported GPIOs configurable

[Jan Kiszka]

On 2017-05-27 15:48, Andy Shevchenko wrote:
> On Fri, May 26, 2017 at 7:02 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> 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.
> 
>> +       unsigned int first_pin;
> 
>> +       if (device_property_read_u32(&pdev->dev, "first_pin", &first_pin) < 0 ||
> 
> And again, we need to follow the rules of the device property
> bindings. (No underscores - use dashes; name should be registered).
> 
> Without Rafael's / Mika's and Rob's opinions I would not go with a
> such name even for internal property.
> 
> DWC3 (USB) for example is still using "linux," prefix. I dunno if it's
> the case here, I mean can or can't we use similar approach.
> 
>> +           device_property_read_u32(&pdev->dev, "npins", &npins) < 0)
> 
> "ngpios"
> 

Yeah, forgot to change all this before reposting. Will make the names
DT-alike.

But there is nothing to register: DT shall stay away from this device.

Jan

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

Re: [PATCH v3 10/10] serial: exar: Add support for IOT2040 device

[Jan Kiszka]

On 2017-05-27 15:52, Andy Shevchenko wrote:
> On Fri, May 26, 2017 at 7:02 PM, 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.
> 
>>  static void *
>> -xr17v35x_register_gpio(struct pci_dev *pcidev, unsigned int first_pin,
>> -                      unsigned int npins)
>> +__xr17v35x_register_gpio(struct pci_dev *pcidev, unsigned int first_pin,
>> +                        unsigned int npins)
> 
>> +static int xr17v35x_register_gpio(struct pci_dev *pcidev,
>> +                                 struct uart_8250_port *port)
>> +{
>> +       if (pcidev->vendor == PCI_VENDOR_ID_EXAR)
>> +               port->port.private_data =
>> +                       __xr17v35x_register_gpio(pcidev, 0, 16);
>> +
>> +       return 0;
>> +}
> 
>> -               if (pcidev->vendor == PCI_VENDOR_ID_EXAR)
>> -                       port->port.private_data =
>> -                               xr17v35x_register_gpio(pcidev, 0, 16);
>> +               ret = platform->register_gpio(pcidev, port);
>>         }
> 
> Can't you move at least some of this to the patch where you move the check?
> 

This change is unrelated to that patch.

What I can offer is a separate patch to factor this out, but I'm not
sure it would buy us much.

Jan

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

Re: [PATCH v3 02/10] gpio-exar/8250-exar: Do not even instantiate a GPIO device for Commtech cards

[Linus Walleij]

On Fri, May 26, 2017 at 6:02 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:

> Commtech adapters need the MPIOs for internal purposes, and the
> gpio-exar driver already refused to pick them up. But there is actually
> no point in even creating the underlying platform device.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v3 03/10] gpio-exar/8250-exar: Fix passing in of parent PCI device

[Linus Walleij]

On Fri, May 26, 2017 at 6:02 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:

> 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.
>
> Set the platform device parent instead.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v3 04/10] gpio: exar: Allocate resources on behalf of the platform device

[Linus Walleij]

On Fri, May 26, 2017 at 6:02 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:

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

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v3 05/10] gpio: exar: Fix iomap request

[Linus Walleij]

On Fri, May 26, 2017 at 6:02 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:

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

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v3 06/10] gpio: exar: Fix reading of directions and values

[Linus Walleij]

On Fri, May 26, 2017 at 6:02 PM, 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.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v3 07/10] gpio-exar/8250-exar: Rearrange gpiochip parenthood

[Linus Walleij]

On Fri, May 26, 2017 at 6:02 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:

> Set the parent of the exar gpiochip to its platform device, like other
> gpiochips are doing it. In order to keep the relationship discoverable
> for ACPI systems, set the platform device companion to the PCI device.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v3 08/10] gpio: exar: Refactor address and bit calculations

[Linus Walleij]

On Fri, May 26, 2017 at 6:02 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:

> Make the address and bit calculation more friendly for introducing a
> "first pin" offset later on. The new form is also more compact and
> regular.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Whether you go with a separate patch or as Andy suggests.

Yours,
Linus Walleij

Re: [PATCH v3 00/10] serial/gpio: exar: Fixes and support for IOT2000

[Linus Walleij]

On Fri, May 26, 2017 at 6:02 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:

> 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 v3:
>  - fix MPIO state for Commtech adapters (regression of merged patch from
>    previous round)
>  - do not create gpio device for Commtech adapters
>  - switch back to device properties
>  - pass parent reference via device.parent instead of platform data
>  - use dmi_system_id table instead of open-coded matching
>  - address some smaller review remarks
>  - fix reading back of rs485 state
>  - adjust parenthood of exar gpiochip

I ACKed some patches if Greg need to merge them.

Can you suggest a merge strategy for this patch set?

It appears Greg will have to merge at least the first one.

Will it work to merge the GPIO patches orthogonally to the GPIO
tree or are they all dependent on the serial changes?

Should I merge them all? Should Greg merge them all?
Should one of us produce an immutable branch?

Yours,
Linus Walleij

Re: [PATCH v3 00/10] serial/gpio: exar: Fixes and support for IOT2000

[Jan Kiszka]

On 2017-05-29 14:39, Linus Walleij wrote:
> On Fri, May 26, 2017 at 6:02 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> 
>> 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 v3:
>>  - fix MPIO state for Commtech adapters (regression of merged patch from
>>    previous round)
>>  - do not create gpio device for Commtech adapters
>>  - switch back to device properties
>>  - pass parent reference via device.parent instead of platform data
>>  - use dmi_system_id table instead of open-coded matching
>>  - address some smaller review remarks
>>  - fix reading back of rs485 state
>>  - adjust parenthood of exar gpiochip
> 
> I ACKed some patches if Greg need to merge them.

Thanks!

> 
> Can you suggest a merge strategy for this patch set?
> 
> It appears Greg will have to merge at least the first one.
> 
> Will it work to merge the GPIO patches orthogonally to the GPIO
> tree or are they all dependent on the serial changes?
> 
> Should I merge them all? Should Greg merge them all?
> Should one of us produce an immutable branch?

Half of the patch are affecting both serial and GPIO subsystem, most
have GPIO focus, though. Just patch 10 or more serial than GPIO. All in
all, maybe Greg can ack and things can flow through the GPIO tree?

Jan

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

Re: [PATCH v3 00/10] serial/gpio: exar: Fixes and support for IOT2000

[Linus Walleij]

On Mon, May 29, 2017 at 3:41 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2017-05-29 14:39, Linus Walleij wrote:

>> Should I merge them all? Should Greg merge them all?
>> Should one of us produce an immutable branch?
>
> Half of the patch are affecting both serial and GPIO subsystem, most
> have GPIO focus, though. Just patch 10 or more serial than GPIO. All in
> all, maybe Greg can ack and things can flow through the GPIO tree?

OK with me, let's see what Greg says.

Yours,
Linus Walleij