linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] gpio: max77620: Use correct unit for debounce times
@ 2019-10-02 12:28 Thierry Reding
  2019-10-02 12:28 ` [PATCH 2/3] gpio: max77620: Do not allocate IRQs upfront Thierry Reding
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Thierry Reding @ 2019-10-02 12:28 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski
  Cc: Timo Alho, linux-gpio, linux-tegra, linux-kernel

From: Thierry Reding <treding@nvidia.com>

The gpiod_set_debounce() function takes the debounce time in
microseconds. Adjust the switch/case values in the MAX77620 GPIO to use
the correct unit.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpio/gpio-max77620.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpio-max77620.c b/drivers/gpio/gpio-max77620.c
index 47d05e357e61..faf86ea9c51a 100644
--- a/drivers/gpio/gpio-max77620.c
+++ b/drivers/gpio/gpio-max77620.c
@@ -192,13 +192,13 @@ static int max77620_gpio_set_debounce(struct max77620_gpio *mgpio,
 	case 0:
 		val = MAX77620_CNFG_GPIO_DBNC_None;
 		break;
-	case 1 ... 8:
+	case 1000 ... 8000:
 		val = MAX77620_CNFG_GPIO_DBNC_8ms;
 		break;
-	case 9 ... 16:
+	case 9000 ... 16000:
 		val = MAX77620_CNFG_GPIO_DBNC_16ms;
 		break;
-	case 17 ... 32:
+	case 17000 ... 32000:
 		val = MAX77620_CNFG_GPIO_DBNC_32ms;
 		break;
 	default:
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] gpio: max77620: Do not allocate IRQs upfront
  2019-10-02 12:28 [PATCH 1/3] gpio: max77620: Use correct unit for debounce times Thierry Reding
@ 2019-10-02 12:28 ` Thierry Reding
  2019-10-04 22:01   ` Linus Walleij
  2019-10-02 12:28 ` [PATCH 3/3] gpio: max77620: Fix interrupt handling Thierry Reding
  2019-10-04 21:58 ` [PATCH 1/3] gpio: max77620: Use correct unit for debounce times Linus Walleij
  2 siblings, 1 reply; 6+ messages in thread
From: Thierry Reding @ 2019-10-02 12:28 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski
  Cc: Timo Alho, linux-gpio, linux-tegra, linux-kernel

From: Thierry Reding <treding@nvidia.com>

regmap_add_irq_chip() will try to allocate all of the IRQ descriptors
upfront if passed a non-zero irq_base parameter. However, the intention
is to allocate IRQ descriptors on an as-needed basis if possible. Pass 0
instead of -1 to fix that use-case.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpio/gpio-max77620.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-max77620.c b/drivers/gpio/gpio-max77620.c
index faf86ea9c51a..c58b56e5291e 100644
--- a/drivers/gpio/gpio-max77620.c
+++ b/drivers/gpio/gpio-max77620.c
@@ -304,7 +304,7 @@ static int max77620_gpio_probe(struct platform_device *pdev)
 	}
 
 	ret = devm_regmap_add_irq_chip(&pdev->dev, chip->rmap, gpio_irq,
-				       IRQF_ONESHOT, -1,
+				       IRQF_ONESHOT, 0,
 				       &max77620_gpio_irq_chip,
 				       &chip->gpio_irq_data);
 	if (ret < 0) {
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] gpio: max77620: Fix interrupt handling
  2019-10-02 12:28 [PATCH 1/3] gpio: max77620: Use correct unit for debounce times Thierry Reding
  2019-10-02 12:28 ` [PATCH 2/3] gpio: max77620: Do not allocate IRQs upfront Thierry Reding
@ 2019-10-02 12:28 ` Thierry Reding
  2019-10-04 22:03   ` Linus Walleij
  2019-10-04 21:58 ` [PATCH 1/3] gpio: max77620: Use correct unit for debounce times Linus Walleij
  2 siblings, 1 reply; 6+ messages in thread
From: Thierry Reding @ 2019-10-02 12:28 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski
  Cc: Timo Alho, linux-gpio, linux-tegra, linux-kernel

From: Timo Alho <talho@nvidia.com>

The interrupt-related register fields on the MAX77620 GPIO controller
share registers with GPIO related fields. If the IRQ chip is implemented
with regmap-irq, this causes the IRQ controller code to overwrite fields
previously configured by the GPIO controller code.

Two examples where this causes problems are the NVIDIA Jetson TX1 and
Jetson TX2 boards, where some of the GPIOs are used to enable vital
power regulators. The MAX77620 GPIO controller also provides the USB OTG
ID pin. If configured as an interrupt, this causes some of the
regulators to be powered off.

Signed-off-by: Timo Alho <talho@nvidia.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpio/gpio-max77620.c | 231 ++++++++++++++++++-----------------
 1 file changed, 117 insertions(+), 114 deletions(-)

diff --git a/drivers/gpio/gpio-max77620.c b/drivers/gpio/gpio-max77620.c
index c58b56e5291e..c5b64a4ac172 100644
--- a/drivers/gpio/gpio-max77620.c
+++ b/drivers/gpio/gpio-max77620.c
@@ -18,109 +18,115 @@ struct max77620_gpio {
 	struct gpio_chip	gpio_chip;
 	struct regmap		*rmap;
 	struct device		*dev;
+	struct mutex		buslock; /* irq_bus_lock */
+	unsigned int		irq_type[8];
+	bool			irq_enabled[8];
 };
 
-static const struct regmap_irq max77620_gpio_irqs[] = {
-	[0] = {
-		.reg_offset = 0,
-		.mask = MAX77620_IRQ_LVL2_GPIO_EDGE0,
-		.type = {
-			.type_rising_val = MAX77620_CNFG_GPIO_INT_RISING,
-			.type_falling_val = MAX77620_CNFG_GPIO_INT_FALLING,
-			.type_reg_mask = MAX77620_CNFG_GPIO_INT_MASK,
-			.type_reg_offset = 0,
-			.types_supported = IRQ_TYPE_EDGE_BOTH,
-		},
-	},
-	[1] = {
-		.reg_offset = 0,
-		.mask = MAX77620_IRQ_LVL2_GPIO_EDGE1,
-		.type = {
-			.type_rising_val = MAX77620_CNFG_GPIO_INT_RISING,
-			.type_falling_val = MAX77620_CNFG_GPIO_INT_FALLING,
-			.type_reg_mask = MAX77620_CNFG_GPIO_INT_MASK,
-			.type_reg_offset = 1,
-			.types_supported = IRQ_TYPE_EDGE_BOTH,
-		},
-	},
-	[2] = {
-		.reg_offset = 0,
-		.mask = MAX77620_IRQ_LVL2_GPIO_EDGE2,
-		.type = {
-			.type_rising_val = MAX77620_CNFG_GPIO_INT_RISING,
-			.type_falling_val = MAX77620_CNFG_GPIO_INT_FALLING,
-			.type_reg_mask = MAX77620_CNFG_GPIO_INT_MASK,
-			.type_reg_offset = 2,
-			.types_supported = IRQ_TYPE_EDGE_BOTH,
-		},
-	},
-	[3] = {
-		.reg_offset = 0,
-		.mask = MAX77620_IRQ_LVL2_GPIO_EDGE3,
-		.type = {
-			.type_rising_val = MAX77620_CNFG_GPIO_INT_RISING,
-			.type_falling_val = MAX77620_CNFG_GPIO_INT_FALLING,
-			.type_reg_mask = MAX77620_CNFG_GPIO_INT_MASK,
-			.type_reg_offset = 3,
-			.types_supported = IRQ_TYPE_EDGE_BOTH,
-		},
-	},
-	[4] = {
-		.reg_offset = 0,
-		.mask = MAX77620_IRQ_LVL2_GPIO_EDGE4,
-		.type = {
-			.type_rising_val = MAX77620_CNFG_GPIO_INT_RISING,
-			.type_falling_val = MAX77620_CNFG_GPIO_INT_FALLING,
-			.type_reg_mask = MAX77620_CNFG_GPIO_INT_MASK,
-			.type_reg_offset = 4,
-			.types_supported = IRQ_TYPE_EDGE_BOTH,
-		},
-	},
-	[5] = {
-		.reg_offset = 0,
-		.mask = MAX77620_IRQ_LVL2_GPIO_EDGE5,
-		.type = {
-			.type_rising_val = MAX77620_CNFG_GPIO_INT_RISING,
-			.type_falling_val = MAX77620_CNFG_GPIO_INT_FALLING,
-			.type_reg_mask = MAX77620_CNFG_GPIO_INT_MASK,
-			.type_reg_offset = 5,
-			.types_supported = IRQ_TYPE_EDGE_BOTH,
-		},
-	},
-	[6] = {
-		.reg_offset = 0,
-		.mask = MAX77620_IRQ_LVL2_GPIO_EDGE6,
-		.type = {
-			.type_rising_val = MAX77620_CNFG_GPIO_INT_RISING,
-			.type_falling_val = MAX77620_CNFG_GPIO_INT_FALLING,
-			.type_reg_mask = MAX77620_CNFG_GPIO_INT_MASK,
-			.type_reg_offset = 6,
-			.types_supported = IRQ_TYPE_EDGE_BOTH,
-		},
-	},
-	[7] = {
-		.reg_offset = 0,
-		.mask = MAX77620_IRQ_LVL2_GPIO_EDGE7,
-		.type = {
-			.type_rising_val = MAX77620_CNFG_GPIO_INT_RISING,
-			.type_falling_val = MAX77620_CNFG_GPIO_INT_FALLING,
-			.type_reg_mask = MAX77620_CNFG_GPIO_INT_MASK,
-			.type_reg_offset = 7,
-			.types_supported = IRQ_TYPE_EDGE_BOTH,
-		},
-	},
-};
+static irqreturn_t max77620_gpio_irqhandler(int irq, void *data)
+{
+	struct max77620_gpio *gpio = data;
+	unsigned int value, offset;
+	unsigned long pending;
+	int err;
+
+	err = regmap_read(gpio->rmap, MAX77620_REG_IRQ_LVL2_GPIO, &value);
+	if (err < 0) {
+		dev_err(gpio->dev, "REG_IRQ_LVL2_GPIO read failed: %d\n", err);
+		return IRQ_NONE;
+	}
+
+	pending = value;
+
+	for_each_set_bit(offset, &pending, 8) {
+		unsigned int virq;
+
+		virq = irq_find_mapping(gpio->gpio_chip.irq.domain, offset);
+		handle_nested_irq(virq);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static void max77620_gpio_irq_mask(struct irq_data *data)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct max77620_gpio *gpio = gpiochip_get_data(chip);
+
+	gpio->irq_enabled[data->hwirq] = false;
+}
 
-static const struct regmap_irq_chip max77620_gpio_irq_chip = {
-	.name = "max77620-gpio",
-	.irqs = max77620_gpio_irqs,
-	.num_irqs = ARRAY_SIZE(max77620_gpio_irqs),
-	.num_regs = 1,
-	.num_type_reg = 8,
-	.irq_reg_stride = 1,
-	.type_reg_stride = 1,
-	.status_base = MAX77620_REG_IRQ_LVL2_GPIO,
-	.type_base = MAX77620_REG_GPIO0,
+static void max77620_gpio_irq_unmask(struct irq_data *data)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct max77620_gpio *gpio = gpiochip_get_data(chip);
+
+	gpio->irq_enabled[data->hwirq] = true;
+}
+
+static int max77620_gpio_set_irq_type(struct irq_data *data, unsigned int type)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct max77620_gpio *gpio = gpiochip_get_data(chip);
+	unsigned int irq_type;
+
+	switch (type) {
+	case IRQ_TYPE_EDGE_RISING:
+		irq_type = MAX77620_CNFG_GPIO_INT_RISING;
+		break;
+
+	case IRQ_TYPE_EDGE_FALLING:
+		irq_type = MAX77620_CNFG_GPIO_INT_FALLING;
+		break;
+
+	case IRQ_TYPE_EDGE_BOTH:
+		irq_type = MAX77620_CNFG_GPIO_INT_RISING |
+			   MAX77620_CNFG_GPIO_INT_FALLING;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	gpio->irq_type[data->hwirq] = irq_type;
+
+	return 0;
+}
+
+static void max77620_gpio_bus_lock(struct irq_data *data)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct max77620_gpio *gpio = gpiochip_get_data(chip);
+
+	mutex_lock(&gpio->buslock);
+}
+
+static void max77620_gpio_bus_sync_unlock(struct irq_data *data)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct max77620_gpio *gpio = gpiochip_get_data(chip);
+	unsigned int value, offset = data->hwirq;
+	int err;
+
+	value = gpio->irq_enabled[offset] ? gpio->irq_type[offset] : 0;
+
+	err = regmap_update_bits(gpio->rmap, GPIO_REG_ADDR(offset),
+				 MAX77620_CNFG_GPIO_INT_MASK, value);
+	if (err < 0)
+		dev_err(chip->parent, "failed to update interrupt mask: %d\n",
+			err);
+
+	mutex_unlock(&gpio->buslock);
+}
+
+static struct irq_chip max77620_gpio_irqchip = {
+	.name		= "max77620-gpio",
+	.irq_mask	= max77620_gpio_irq_mask,
+	.irq_unmask	= max77620_gpio_irq_unmask,
+	.irq_set_type	= max77620_gpio_set_irq_type,
+	.irq_bus_lock	= max77620_gpio_bus_lock,
+	.irq_bus_sync_unlock = max77620_gpio_bus_sync_unlock,
+	.flags		= IRQCHIP_MASK_ON_SUSPEND,
 };
 
 static int max77620_gpio_dir_input(struct gpio_chip *gc, unsigned int offset)
@@ -254,14 +260,6 @@ static int max77620_gpio_set_config(struct gpio_chip *gc, unsigned int offset,
 	return -ENOTSUPP;
 }
 
-static int max77620_gpio_to_irq(struct gpio_chip *gc, unsigned int offset)
-{
-	struct max77620_gpio *mgpio = gpiochip_get_data(gc);
-	struct max77620_chip *chip = dev_get_drvdata(mgpio->dev->parent);
-
-	return regmap_irq_get_virq(chip->gpio_irq_data, offset);
-}
-
 static int max77620_gpio_probe(struct platform_device *pdev)
 {
 	struct max77620_chip *chip =  dev_get_drvdata(pdev->dev.parent);
@@ -287,7 +285,6 @@ static int max77620_gpio_probe(struct platform_device *pdev)
 	mgpio->gpio_chip.direction_output = max77620_gpio_dir_output;
 	mgpio->gpio_chip.set = max77620_gpio_set;
 	mgpio->gpio_chip.set_config = max77620_gpio_set_config;
-	mgpio->gpio_chip.to_irq = max77620_gpio_to_irq;
 	mgpio->gpio_chip.ngpio = MAX77620_GPIO_NR;
 	mgpio->gpio_chip.can_sleep = 1;
 	mgpio->gpio_chip.base = -1;
@@ -303,15 +300,21 @@ static int max77620_gpio_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	ret = devm_regmap_add_irq_chip(&pdev->dev, chip->rmap, gpio_irq,
-				       IRQF_ONESHOT, 0,
-				       &max77620_gpio_irq_chip,
-				       &chip->gpio_irq_data);
+	mutex_init(&mgpio->buslock);
+
+	gpiochip_irqchip_add_nested(&mgpio->gpio_chip, &max77620_gpio_irqchip,
+				    0, handle_edge_irq, IRQ_TYPE_NONE);
+
+	ret = request_threaded_irq(gpio_irq, NULL, max77620_gpio_irqhandler,
+				   IRQF_ONESHOT, "max77620-gpio", mgpio);
 	if (ret < 0) {
-		dev_err(&pdev->dev, "Failed to add gpio irq_chip %d\n", ret);
+		dev_err(&pdev->dev, "failed to request IRQ: %d\n", ret);
 		return ret;
 	}
 
+	gpiochip_set_nested_irqchip(&mgpio->gpio_chip, &max77620_gpio_irqchip,
+				    gpio_irq);
+
 	return 0;
 }
 
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] gpio: max77620: Use correct unit for debounce times
  2019-10-02 12:28 [PATCH 1/3] gpio: max77620: Use correct unit for debounce times Thierry Reding
  2019-10-02 12:28 ` [PATCH 2/3] gpio: max77620: Do not allocate IRQs upfront Thierry Reding
  2019-10-02 12:28 ` [PATCH 3/3] gpio: max77620: Fix interrupt handling Thierry Reding
@ 2019-10-04 21:58 ` Linus Walleij
  2 siblings, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2019-10-04 21:58 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Bartosz Golaszewski, Timo Alho, open list:GPIO SUBSYSTEM,
	linux-tegra, linux-kernel

On Wed, Oct 2, 2019 at 2:28 PM Thierry Reding <thierry.reding@gmail.com> wrote:

> From: Thierry Reding <treding@nvidia.com>
>
> The gpiod_set_debounce() function takes the debounce time in
> microseconds. Adjust the switch/case values in the MAX77620 GPIO to use
> the correct unit.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Patch applied for fixes.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/3] gpio: max77620: Do not allocate IRQs upfront
  2019-10-02 12:28 ` [PATCH 2/3] gpio: max77620: Do not allocate IRQs upfront Thierry Reding
@ 2019-10-04 22:01   ` Linus Walleij
  0 siblings, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2019-10-04 22:01 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Bartosz Golaszewski, Timo Alho, open list:GPIO SUBSYSTEM,
	linux-tegra, linux-kernel

On Wed, Oct 2, 2019 at 2:28 PM Thierry Reding <thierry.reding@gmail.com> wrote:

> From: Thierry Reding <treding@nvidia.com>
>
> regmap_add_irq_chip() will try to allocate all of the IRQ descriptors
> upfront if passed a non-zero irq_base parameter. However, the intention
> is to allocate IRQ descriptors on an as-needed basis if possible. Pass 0
> instead of -1 to fix that use-case.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Patch applied.

Yours,
Linus Walleij

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] gpio: max77620: Fix interrupt handling
  2019-10-02 12:28 ` [PATCH 3/3] gpio: max77620: Fix interrupt handling Thierry Reding
@ 2019-10-04 22:03   ` Linus Walleij
  0 siblings, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2019-10-04 22:03 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Bartosz Golaszewski, Timo Alho, open list:GPIO SUBSYSTEM,
	linux-tegra, linux-kernel

On Wed, Oct 2, 2019 at 2:28 PM Thierry Reding <thierry.reding@gmail.com> wrote:

> From: Timo Alho <talho@nvidia.com>
>
> The interrupt-related register fields on the MAX77620 GPIO controller
> share registers with GPIO related fields. If the IRQ chip is implemented
> with regmap-irq, this causes the IRQ controller code to overwrite fields
> previously configured by the GPIO controller code.
>
> Two examples where this causes problems are the NVIDIA Jetson TX1 and
> Jetson TX2 boards, where some of the GPIOs are used to enable vital
> power regulators. The MAX77620 GPIO controller also provides the USB OTG
> ID pin. If configured as an interrupt, this causes some of the
> regulators to be powered off.
>
> Signed-off-by: Timo Alho <talho@nvidia.com>
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Patch applied.

I am looking for ways to pass also nested irqchips along when registering
the gpio_chip but I guess I need to clean up all the chained chips
first (getting there...)

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-10-04 22:03 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-02 12:28 [PATCH 1/3] gpio: max77620: Use correct unit for debounce times Thierry Reding
2019-10-02 12:28 ` [PATCH 2/3] gpio: max77620: Do not allocate IRQs upfront Thierry Reding
2019-10-04 22:01   ` Linus Walleij
2019-10-02 12:28 ` [PATCH 3/3] gpio: max77620: Fix interrupt handling Thierry Reding
2019-10-04 22:03   ` Linus Walleij
2019-10-04 21:58 ` [PATCH 1/3] gpio: max77620: Use correct unit for debounce times Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).