gpio: exar: refactor the driver

PATCH In review Bartosz Golaszewski 3 participants 16 messages Nov 16, 2020 – Nov 23, 2020 GPIO

lore

Share & tools

lore

Bartosz Golaszewski

author

2020-11-16 12:01 UTC #0 lore raw

v5 cover gpio: exar: refactor the driver

From: Bartosz Golaszewski <[email protected]>

I just wanted to convert the driver to using simpler IDA API but ended up quickly converting it to using regmap. Unfortunately I don't have the HW to test it so marking the patches that introduce functional change as RFT and Cc'ing the original author.

v1 -> v2:
- add new regmap helper: regmap_assign_bits()
- fix lvl vs sel register access
- set value in direction_output callback

v2 -> v3:
- drop the regmap helper from series

v3 -> v4:
- renamed the regmap variable to 'regmap' as leaving the old name caused me to miss an assignment leading to a crash (culprit spotted by Andy Shevchenko)

v4 -> v5:
- set the reg_bits to 11 in regmap config
- collect review tags from Andy

Bartosz Golaszewski (7):
gpio: exar: add a newline after the copyright notice
gpio: exar: include idr.h
gpio: exar: switch to a simpler IDA interface
gpio: exar: use a helper variable for &pdev->dev
gpio: exar: unduplicate address and offset computation
gpio: exar: switch to using regmap
gpio: exar: use devm action for freeing the IDA and drop remove()

 drivers/gpio/Kconfig     |   1 +
 drivers/gpio/gpio-exar.c | 160 ++++++++++++++++++++-------------------
 2 files changed, 82 insertions(+), 79 deletions(-)

Signature · 1 line

2.29.1

Bartosz Golaszewski

author

2020-11-16 12:01 UTC #1 lore raw

PATCH v5 1/7 gpio: exar: add a newline after the copyright notice

From: Bartosz Golaszewski <[email protected]>

It's customary to have a newline between the copyright header and the includes. Add one to gpio-exar.

Signed-off-by Bartosz Golaszewski <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

---
 drivers/gpio/gpio-exar.c | 1 +
 1 file changed, 1 insertion(+)

drivers/gpio/gpio-exar.c +1 -0

index b1accfba017d..4202dd363a11 100644
@@ -4,6 +4,7 @@
  *
  * Copyright (C) 2015 Sudip Mukherjee <[email protected]>
  */
+
 #include <linux/bitops.h>
 #include <linux/device.h>
 #include <linux/gpio/driver.h>

Signature · 1 line

2.29.1

Bartosz Golaszewski

author

2020-11-16 12:02 UTC #2 lore raw

PATCH v5 2/7 gpio: exar: include idr.h

In reply to Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

This driver uses IDA APIs but doesn't include the relevant header. This fixes it.

Signed-off-by Bartosz Golaszewski <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

---
 drivers/gpio/gpio-exar.c | 1 +
 1 file changed, 1 insertion(+)

drivers/gpio/gpio-exar.c +1 -0

index 4202dd363a11..1941ae533418 100644
@@ -8,6 +8,7 @@
 #include <linux/bitops.h>
 #include <linux/device.h>
 #include <linux/gpio/driver.h>
+#include <linux/idr.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/module.h>

Signature · 1 line

2.29.1

Bartosz Golaszewski

author

2020-11-16 12:01 UTC #3 lore raw

PATCH v5 3/7 gpio: exar: switch to a simpler IDA interface

In reply to Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

We don't need to specify any ranges when allocating IDs so we can switch to ida_alloc() and ida_free() instead of the ida_simple_ counterparts.

ida_simple_get(ida, 0, 0, gfp) is equivalent to ida_alloc_range(ida, 0, UINT_MAX, gfp) which is equivalent to ida_alloc(ida, gfp). Note: IDR will never actually allocate an ID larger than INT_MAX.

Signed-off-by Bartosz Golaszewski <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

---
 drivers/gpio/gpio-exar.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

drivers/gpio/gpio-exar.c +3 -3

index 1941ae533418..752e8437ff80 100644
@@ -149,7 +149,7 @@ static int gpio_exar_probe(struct platform_device *pdev)
 
 	mutex_init(&exar_gpio->lock);
 
-	index = ida_simple_get(&ida_index, 0, 0, GFP_KERNEL);
+	index = ida_alloc(&ida_index, GFP_KERNEL);
 	if (index < 0) {
 		ret = index;
 		goto err_mutex_destroy;

@@ -179,7 +179,7 @@ static int gpio_exar_probe(struct platform_device *pdev)
 	return 0;
 
 err_destroy:
-	ida_simple_remove(&ida_index, index);
+	ida_free(&ida_index, index);
 err_mutex_destroy:
 	mutex_destroy(&exar_gpio->lock);
 	return ret;

@@ -189,7 +189,7 @@ static int gpio_exar_remove(struct platform_device *pdev)
 {
 	struct exar_gpio_chip *exar_gpio = platform_get_drvdata(pdev);
 
-	ida_simple_remove(&ida_index, exar_gpio->index);
+	ida_free(&ida_index, exar_gpio->index);
 	mutex_destroy(&exar_gpio->lock);
 
 	return 0;

Signature · 1 line

2.29.1

Bartosz Golaszewski

author

2020-11-16 12:02 UTC #4 lore raw

PATCH v5 4/7 gpio: exar: use a helper variable for &pdev->dev

In reply to Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

It's more elegant to use a helper local variable to store the address of the underlying struct device than to dereference pdev everywhere. It also has the benefit of avoiding unnecessary line breaks.

Signed-off-by Bartosz Golaszewski <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

---
 drivers/gpio/gpio-exar.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

drivers/gpio/gpio-exar.c +7 -8

index 752e8437ff80..db366d85b6b4 100644
@@ -120,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 = to_pci_dev(pdev->dev.parent);
+	struct device *dev = &pdev->dev;
+	struct pci_dev *pcidev = to_pci_dev(dev->parent);
 	struct exar_gpio_chip *exar_gpio;
 	u32 first_pin, ngpios;
 	void __iomem *p;

@@ -134,16 +135,15 @@ static int gpio_exar_probe(struct platform_device *pdev)
 	if (!p)
 		return -ENOMEM;
 
-	ret = device_property_read_u32(&pdev->dev, "exar,first-pin",
-				       &first_pin);
+	ret = device_property_read_u32(dev, "exar,first-pin", &first_pin);
 	if (ret)
 		return ret;
 
-	ret = device_property_read_u32(&pdev->dev, "ngpios", &ngpios);
+	ret = device_property_read_u32(dev, "ngpios", &ngpios);
 	if (ret)
 		return ret;
 
-	exar_gpio = devm_kzalloc(&pdev->dev, sizeof(*exar_gpio), GFP_KERNEL);
+	exar_gpio = devm_kzalloc(dev, sizeof(*exar_gpio), GFP_KERNEL);
 	if (!exar_gpio)
 		return -ENOMEM;

@@ -157,7 +157,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 = &pdev->dev;
+	exar_gpio->gpio_chip.parent = 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;

@@ -169,8 +169,7 @@ static int gpio_exar_probe(struct platform_device *pdev)
 	exar_gpio->index = index;
 	exar_gpio->first_pin = first_pin;
 
-	ret = devm_gpiochip_add_data(&pdev->dev,
-				     &exar_gpio->gpio_chip, exar_gpio);
+	ret = devm_gpiochip_add_data(dev, &exar_gpio->gpio_chip, exar_gpio);
 	if (ret)
 		goto err_destroy;

Signature · 1 line

2.29.1

Bartosz Golaszewski

author

2020-11-16 12:03 UTC #5 lore raw

PATCH v5 5/7 gpio: exar: unduplicate address and offset computation

In reply to Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

Provide and use helpers for calculating the register address and bit offset instead of hand coding it in every function.

Signed-off-by Bartosz Golaszewski <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

---
 drivers/gpio/gpio-exar.c | 40 ++++++++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 12 deletions(-)

drivers/gpio/gpio-exar.c +28 ~~-12~~

index db366d85b6b4..28b0b4b5fa35 100644
@@ -33,6 +33,26 @@ struct exar_gpio_chip {
 	unsigned int first_pin;
 };
 
+static unsigned int
+exar_offset_to_sel_addr(struct exar_gpio_chip *exar_gpio, unsigned int offset)
+{
+	return (offset + exar_gpio->first_pin) / 8 ? EXAR_OFFSET_MPIOSEL_HI
+						   : EXAR_OFFSET_MPIOSEL_LO;
+}
+
+static unsigned int
+exar_offset_to_lvl_addr(struct exar_gpio_chip *exar_gpio, unsigned int offset)
+{
+	return (offset + exar_gpio->first_pin) / 8 ? EXAR_OFFSET_MPIOLVL_HI
+						   : EXAR_OFFSET_MPIOLVL_LO;
+}
+
+static unsigned int
+exar_offset_to_bit(struct exar_gpio_chip *exar_gpio, unsigned int offset)
+{
+	return (offset + exar_gpio->first_pin) % 8;
+}
+
 static void exar_update(struct gpio_chip *chip, unsigned int reg, int val,
 			unsigned int offset)
 {

@@ -52,9 +72,8 @@ static int exar_set_direction(struct gpio_chip *chip, int direction,
 			      unsigned int offset)
 {
 	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 + exar_gpio->first_pin) % 8;
+	unsigned int addr = exar_offset_to_sel_addr(exar_gpio, offset);
+	unsigned int bit = exar_offset_to_bit(exar_gpio, offset);
 
 	exar_update(chip, addr, direction, bit);
 	return 0;

@@ -75,9 +94,8 @@ static int exar_get(struct gpio_chip *chip, unsigned int reg)
 static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
 {
 	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 + exar_gpio->first_pin) % 8;
+	unsigned int addr = exar_offset_to_sel_addr(exar_gpio, offset);
+	unsigned int bit = exar_offset_to_bit(exar_gpio, offset);
 
 	if (exar_get(chip, addr) & BIT(bit))
 		return GPIO_LINE_DIRECTION_IN;

@@ -88,9 +106,8 @@ static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
 static int exar_get_value(struct gpio_chip *chip, unsigned int offset)
 {
 	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 + exar_gpio->first_pin) % 8;
+	unsigned int addr = exar_offset_to_lvl_addr(exar_gpio, offset);
+	unsigned int bit = exar_offset_to_bit(exar_gpio, offset);
 
 	return !!(exar_get(chip, addr) & BIT(bit));
 }

@@ -99,9 +116,8 @@ static void exar_set_value(struct gpio_chip *chip, unsigned int offset,
 			   int value)
 {
 	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 + exar_gpio->first_pin) % 8;
+	unsigned int addr = exar_offset_to_lvl_addr(exar_gpio, offset);
+	unsigned int bit = exar_offset_to_bit(exar_gpio, offset);
 
 	exar_update(chip, addr, value, bit);
 }

Signature · 1 line

2.29.1

Bartosz Golaszewski

author

2020-11-16 12:03 UTC #6 lore raw

PATCH v5 6/7 gpio: exar: switch to using regmap

In reply to Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

We can simplify the code in gpio-exar by using regmap. This allows us to drop the mutex (regmap provides its own locking) and we can also reuse regmap's bit operations instead of implementing our own update function.

Signed-off-by Bartosz Golaszewski <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

---
 drivers/gpio/Kconfig     |  1 +
 drivers/gpio/gpio-exar.c | 96 ++++++++++++++++++----------------------
 2 files changed, 43 insertions(+), 54 deletions(-)

drivers/gpio/Kconfig +1 -0

index 5d4de5cd6759..253a61ec9645 100644
@@ -255,6 +255,7 @@ config GPIO_EP93XX
 config GPIO_EXAR
 	tristate "Support for GPIO pins on XR17V352/354/358"
 	depends on SERIAL_8250_EXAR
+	select REGMAP_MMIO
 	help
 	  Selecting this option will enable handling of GPIO pins present
 	  on Exar XR17V352/354/358 chips.

drivers/gpio/gpio-exar.c +42 ~~-54~~

171 lines not on this page · raw message

Signature · 1 line

2.29.1

Bartosz Golaszewski

author

2020-11-16 12:03 UTC #7 lore raw

PATCH v5 7/7 gpio: exar: use devm action for freeing the IDA and drop remove()

In reply to Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

We can simplify the error path in probe() and drop remove() entirely if we provide a devm action for freeing the device ID.

Signed-off-by Bartosz Golaszewski <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

---
 drivers/gpio/gpio-exar.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

drivers/gpio/gpio-exar.c +12 ~~-15~~

index 79fb0964ace3..e4c1ac88d2ba 100644
@@ -111,6 +111,13 @@ static int exar_direction_input(struct gpio_chip *chip, unsigned int offset)
 	return 0;
 }
 
+static void exar_devm_ida_free(void *data)
+{
+	struct exar_gpio_chip *exar_gpio = data;
+
+	ida_free(&ida_index, exar_gpio->index);
+}
+
 static const struct regmap_config exar_regmap_config = {
 	.name		= "exar-gpio",
 	/*

@@ -163,6 +170,10 @@ static int gpio_exar_probe(struct platform_device *pdev)
 	if (index < 0)
 		return index;
 
+	ret = devm_add_action_or_reset(dev, exar_devm_ida_free, exar_gpio);
+	if (ret)
+		return ret;
+
 	sprintf(exar_gpio->name, "exar_gpio%d", index);
 	exar_gpio->gpio_chip.label = exar_gpio->name;
 	exar_gpio->gpio_chip.parent = dev;

@@ -178,29 +189,15 @@ static int gpio_exar_probe(struct platform_device *pdev)
 
 	ret = devm_gpiochip_add_data(dev, &exar_gpio->gpio_chip, exar_gpio);
 	if (ret)
-		goto err_destroy;
+		return ret;
 
 	platform_set_drvdata(pdev, exar_gpio);
 
-	return 0;
-
-err_destroy:
-	ida_free(&ida_index, index);
-	return ret;
-}
-
-static int gpio_exar_remove(struct platform_device *pdev)
-{
-	struct exar_gpio_chip *exar_gpio = platform_get_drvdata(pdev);
-
-	ida_free(&ida_index, exar_gpio->index);
-
 	return 0;
 }
 
 static struct platform_driver gpio_exar_driver = {
 	.probe	= gpio_exar_probe,
-	.remove	= gpio_exar_remove,
 	.driver	= {
 		.name = DRIVER_NAME,
 	},

Signature · 1 line

2.29.1

Bartosz Golaszewski

author

2020-11-23 11:38 UTC #8 lore raw

In reply to Bartosz Golaszewski

Quoted 8 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>
I just wanted to convert the driver to using simpler IDA API but ended up quickly converting it to using regmap. Unfortunately I don't have the HW to test it so marking the patches that introduce functional change as RFT and Cc'ing the original author.

Hi Jan!

Could you give this last version a final spin before I merge it?

Bartosz

Jan Kiszka

2020-11-23 11:59 UTC #9 lore raw

Quoted 10 lines · Bartosz Golaszewski

Quoted 8 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
I just wanted to convert the driver to using simpler IDA API but ended up quickly converting it to using regmap. Unfortunately I don't have the HW to test it so marking the patches that introduce functional change as RFT and Cc'ing the original author.

Quoted 4 lines

Hi Jan!
Could you give this last version a final spin before I merge it?

[   14.250117] exar_serial 0000:02:00.0: enabling device (0000 -> 0002)
[   14.336622] 0000:02:00.0: ttyS2 at MMIO 0x90000000 (irq = 44, base_baud = 7812500) is a XR17V35X
[   14.391588] 0000:02:00.0: ttyS3 at MMIO 0x90000400 (irq = 44, base_baud = 7812500) is a XR17V35X
[   19.250510] gpio_exar: probe of gpio_exar.1.auto failed with error -22

That's "new"...

Jan

Signature · 2 lines

Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

Jan Kiszka

2020-11-23 12:08 UTC #10 lore raw

Quoted 16 lines · Jan Kiszka

Quoted 10 lines · Bartosz Golaszewski
Quoted 8 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
I just wanted to convert the driver to using simpler IDA API but ended up quickly converting it to using regmap. Unfortunately I don't have the HW to test it so marking the patches that introduce functional change as RFT and Cc'ing the original author.
Quoted 4 lines
Hi Jan!
Could you give this last version a final spin before I merge it?

Quoted 7 lines

[   14.250117] exar_serial 0000:02:00.0: enabling device (0000 -> 0002)
[   14.336622] 0000:02:00.0: ttyS2 at MMIO 0x90000000 (irq = 44, base_baud = 7812500) is a XR17V35X
[   14.391588] 0000:02:00.0: ttyS3 at MMIO 0x90000400 (irq = 44, base_baud = 7812500) is a XR17V35X
[   19.250510] gpio_exar: probe of gpio_exar.1.auto failed with error -22

That's "new"...

Bisected to "gpio: exar: switch to using regmap" again.

Jan

Signature · 2 lines

Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

Bartosz Golaszewski

author

2020-11-23 13:04 UTC #11 lore raw

Quoted 23 lines · Jan Kiszka

Quoted 21 lines · Jan Kiszka
Quoted 10 lines · Bartosz Golaszewski
Quoted 8 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
I just wanted to convert the driver to using simpler IDA API but ended up quickly converting it to using regmap. Unfortunately I don't have the HW to test it so marking the patches that introduce functional change as RFT and Cc'ing the original author.
Quoted 4 lines
Hi Jan!
Could you give this last version a final spin before I merge it?
[   14.250117] exar_serial 0000:02:00.0: enabling device (0000 -> 0002)
[   14.336622] 0000:02:00.0: ttyS2 at MMIO 0x90000000 (irq = 44, base_baud = 7812500) is a XR17V35X
[   14.391588] 0000:02:00.0: ttyS3 at MMIO 0x90000400 (irq = 44, base_baud = 7812500) is a XR17V35X
[   19.250510] gpio_exar: probe of gpio_exar.1.auto failed with error -22

Quoted 5 lines

Quoted 2 lines
That's "new"...
Bisected to "gpio: exar: switch to using regmap" again.

I'm not sure if you saw my email which I sent at the same time as you
- but does reverting reg_bits to 16 help?

Bart

Bartosz Golaszewski

author

2020-11-23 12:12 UTC #12 lore raw

In reply to Jan Kiszka

Thanks!On Mon, Nov 23, 2020 at 1:03 PM Jan Kiszka

Quoted 17 lines · <[email protected]> wrote:

Quoted 10 lines · Bartosz Golaszewski
Quoted 8 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
I just wanted to convert the driver to using simpler IDA API but ended up quickly converting it to using regmap. Unfortunately I don't have the HW to test it so marking the patches that introduce functional change as RFT and Cc'ing the original author.
Quoted 4 lines
Hi Jan!
Could you give this last version a final spin before I merge it?

Quoted 7 lines

[   14.250117] exar_serial 0000:02:00.0: enabling device (0000 -> 0002)
[   14.336622] 0000:02:00.0: ttyS2 at MMIO 0x90000000 (irq = 44, base_baud = 7812500) is a XR17V35X
[   14.391588] 0000:02:00.0: ttyS3 at MMIO 0x90000400 (irq = 44, base_baud = 7812500) is a XR17V35X
[   19.250510] gpio_exar: probe of gpio_exar.1.auto failed with error -22

That's "new"...

And if you change reg_bits from 11 to 16?

Bartosz

Jan Kiszka

2020-11-23 12:57 UTC #13 lore raw

Quoted 24 lines · Bartosz Golaszewski

Thanks!On Mon, Nov 23, 2020 at 1:03 PM Jan Kiszka
Quoted 22 lines · <[email protected]> wrote:
Quoted 10 lines · Bartosz Golaszewski
Quoted 8 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
I just wanted to convert the driver to using simpler IDA API but ended up quickly converting it to using regmap. Unfortunately I don't have the HW to test it so marking the patches that introduce functional change as RFT and Cc'ing the original author.
Quoted 4 lines
Hi Jan!
Could you give this last version a final spin before I merge it?
[   14.250117] exar_serial 0000:02:00.0: enabling device (0000 -> 0002)
[   14.336622] 0000:02:00.0: ttyS2 at MMIO 0x90000000 (irq = 44, base_baud = 7812500) is a XR17V35X
[   14.391588] 0000:02:00.0: ttyS3 at MMIO 0x90000400 (irq = 44, base_baud = 7812500) is a XR17V35X
[   19.250510] gpio_exar: probe of gpio_exar.1.auto failed with error -22

Quoted 5 lines

Quoted 2 lines
That's "new"...
And if you change reg_bits from 11 to 16?

16 works. Didn't we have that already?

Jan

Signature · 2 lines

Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

Bartosz Golaszewski

author

2020-11-23 13:33 UTC #14 lore raw

Quoted 29 lines · Jan Kiszka

Quoted 27 lines · Bartosz Golaszewski
Thanks!On Mon, Nov 23, 2020 at 1:03 PM Jan Kiszka
Quoted 24 lines · <[email protected]> wrote:
Quoted 10 lines · Bartosz Golaszewski
Quoted 8 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
I just wanted to convert the driver to using simpler IDA API but ended up quickly converting it to using regmap. Unfortunately I don't have the HW to test it so marking the patches that introduce functional change as RFT and Cc'ing the original author.
Quoted 4 lines
Hi Jan!
Could you give this last version a final spin before I merge it?
[   14.250117] exar_serial 0000:02:00.0: enabling device (0000 -> 0002)
[   14.336622] 0000:02:00.0: ttyS2 at MMIO 0x90000000 (irq = 44, base_baud = 7812500) is a XR17V35X
[   14.391588] 0000:02:00.0: ttyS3 at MMIO 0x90000400 (irq = 44, base_baud = 7812500) is a XR17V35X
[   19.250510] gpio_exar: probe of gpio_exar.1.auto failed with error -22
That's "new"...

Quoted 6 lines

Quoted 2 lines
And if you change reg_bits from 11 to 16?
16 works. Didn't we have that already?
Jan

Yes we have, Andy suggested 11 is fine because it fits the highest address we need to access but it seems regmap doesn't like this value. Ok so I'll change it when applying, is that fine with you?

Bart

Andy Shevchenko

2020-11-23 13:36 UTC #15 lore raw

In reply to Jan Kiszka

Quoted 25 lines · Jan Kiszka

Quoted 24 lines · Bartosz Golaszewski
Thanks!On Mon, Nov 23, 2020 at 1:03 PM Jan Kiszka
Quoted 22 lines · <[email protected]> wrote:
Quoted 10 lines · Bartosz Golaszewski
Quoted 8 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
I just wanted to convert the driver to using simpler IDA API but ended up quickly converting it to using regmap. Unfortunately I don't have the HW to test it so marking the patches that introduce functional change as RFT and Cc'ing the original author.
Quoted 4 lines
Hi Jan!
Could you give this last version a final spin before I merge it?
[   14.250117] exar_serial 0000:02:00.0: enabling device (0000 -> 0002)
[   14.336622] 0000:02:00.0: ttyS2 at MMIO 0x90000000 (irq = 44, base_baud = 7812500) is a XR17V35X
[   14.391588] 0000:02:00.0: ttyS3 at MMIO 0x90000400 (irq = 44, base_baud = 7812500) is a XR17V35X
[   19.250510] gpio_exar: probe of gpio_exar.1.auto failed with error -22

Quoted 7 lines

Quoted 5 lines
Quoted 2 lines
That's "new"...
And if you change reg_bits from 11 to 16?
16 works. Didn't we have that already?

Yes, but it puzzles me what the idea behind it and don't we see the regmap issue here. In any case, I'm fine with 16 there, although it's not clear to me why 11 is not working...

Signature · 2 lines

With Best Regards,
Andy Shevchenko