Skip to contentSkip to conversation

gpio: sim: lock simulated GPIOs as interrupts

PATCH Applied Bartosz Golaszewski 2 participants 8 messages Jun 24, 2024 – Jul 4, 2024

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

v2 cover gpio: sim: lock simulated GPIOs as interrupts

From: Bartosz Golaszewski <[email protected]>

I realized that the gpio-sim module doesn't lock the GPIOs as interrupts when they are requested from the irq_sim. This leads to users being able to change the direction of GPIOs that should remain as inputs to output. This series extends the irq_sim interface and allows users to supply callbacks that will be executed to inform users about interrupts being requested and released so that they can act accordingly. The gpio-sim is made to use this new API and lock GPIOs as interrupts when needed.

Thomas: if this is fine with you, can you Ack it so that I can take it through the GPIO tree for the next merge window?

Changes since v1:
- drop the notifier in favor of specific callbacks

Bartosz Golaszewski (2):
genirq/irq_sim: add an extended irq_sim initializer
gpio: sim: lock GPIOs as interrupts when they are requested

 drivers/gpio/gpio-sim.c | 25 ++++++++++++++++-
 include/linux/irq_sim.h | 17 ++++++++++++
 kernel/irq/irq_sim.c    | 60 ++++++++++++++++++++++++++++++++++++++---
 3 files changed, 98 insertions(+), 4 deletions(-)
Signature · 1 line
2.43.0
#1 lore raw

PATCH v2 1/2 genirq/irq_sim: add an extended irq_sim initializer

From: Bartosz Golaszewski <[email protected]>

Currently users of the interrupt simulator don't have any way of being notified about interrupts from the simulated domain being requested or released. This causes a problem for one of the users - the GPIO simulator - which is unable to lock the pins as interrupts.

Define a structure containing callbacks to be executed on various irq_sim-related events (for now: irq request and release) and provide an extended function for creating simulated interrupt domains that takes it and a pointer to custom user data (to be passed to said callbacks) as arguments.

---
 include/linux/irq_sim.h | 17 ++++++++++++
 kernel/irq/irq_sim.c    | 60 ++++++++++++++++++++++++++++++++++++++---
 2 files changed, 74 insertions(+), 3 deletions(-)
include/linux/irq_sim.h +17 -0
index ab831e5ae748..89b4d8ff274b 100644
@@ -16,11 +16,28 @@
  * requested like normal irqs and enqueued from process context.
  */
 
+struct irq_sim_ops {
+	int (*irq_sim_irq_requested)(struct irq_domain *domain,
+				     irq_hw_number_t hwirq, void *data);
+	void (*irq_sim_irq_released)(struct irq_domain *domain,
+				     irq_hw_number_t hwirq, void *data);
+};
+
 struct irq_domain *irq_domain_create_sim(struct fwnode_handle *fwnode,
 					 unsigned int num_irqs);
 struct irq_domain *devm_irq_domain_create_sim(struct device *dev,
 					      struct fwnode_handle *fwnode,
 					      unsigned int num_irqs);
+struct irq_domain *irq_domain_create_sim_full(struct fwnode_handle *fwnode,
+					      unsigned int num_irqs,
+					      const struct irq_sim_ops *ops,
+					      void *data);
+struct irq_domain *
+devm_irq_domain_create_sim_full(struct device *dev,
+				struct fwnode_handle *fwnode,
+				unsigned int num_irqs,
+				const struct irq_sim_ops *ops,
+				void *data);
 void irq_domain_remove_sim(struct irq_domain *domain);
 
 #endif /* _LINUX_IRQ_SIM_H */
kernel/irq/irq_sim.c +57 -3

106 lines not on this page · raw message

Signature · 1 line
2.43.0
Reviewed-by on v2 1/2
#2 lore raw

Re: [PATCH v2 1/2] genirq/irq_sim: add an extended irq_sim initializer

Quoted 7 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

Currently users of the interrupt simulator don't have any way of being notified about interrupts from the simulated domain being requested or released. This causes a problem for one of the users - the GPIO simulator - which is unable to lock the pins as interrupts.

Quoted 7 lines

Define a structure containing callbacks to be executed on various irq_sim-related events (for now: irq request and release) and provide an extended function for creating simulated interrupt domains that takes it and a pointer to custom user data (to be passed to said callbacks) as arguments.

Fair enough, I see you agreed with tglx on this layout so:

Yours,
Linus Walleij

#3 lore raw

PATCH v2 2/2 gpio: sim: lock GPIOs as interrupts when they are requested

In reply to Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

Use the extended irq_sim interface to supply the simulated interrupt domain with callbacks allowing the GPIO sim to lock/unlock GPIOs requested as interrupts.

---
 drivers/gpio/gpio-sim.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)
drivers/gpio/gpio-sim.c +24 -1
index 4157735ea791..dcca1d7f173e 100644
@@ -227,6 +227,27 @@ static void gpio_sim_free(struct gpio_chip *gc, unsigned int offset)
 	}
 }
 
+static int gpio_sim_irq_requested(struct irq_domain *domain,
+				  irq_hw_number_t hwirq, void *data)
+{
+	struct gpio_sim_chip *chip = data;
+
+	return gpiochip_lock_as_irq(&chip->gc, hwirq);
+}
+
+static void gpio_sim_irq_released(struct irq_domain *domain,
+				  irq_hw_number_t hwirq, void *data)
+{
+	struct gpio_sim_chip *chip = data;
+
+	gpiochip_unlock_as_irq(&chip->gc, hwirq);
+}
+
+static const struct irq_sim_ops gpio_sim_irq_sim_ops = {
+	.irq_sim_irq_requested = gpio_sim_irq_requested,
+	.irq_sim_irq_released = gpio_sim_irq_released,
+};
+
 static void gpio_sim_dbg_show(struct seq_file *seq, struct gpio_chip *gc)
 {
 	struct gpio_sim_chip *chip = gpiochip_get_data(gc);
@@ -443,7 +464,9 @@ static int gpio_sim_add_bank(struct fwnode_handle *swnode, struct device *dev)
 	if (!chip->pull_map)
 		return -ENOMEM;
 
-	chip->irq_sim = devm_irq_domain_create_sim(dev, swnode, num_lines);
+	chip->irq_sim = devm_irq_domain_create_sim_full(dev, swnode, num_lines,
+							&gpio_sim_irq_sim_ops,
+							chip);
 	if (IS_ERR(chip->irq_sim))
 		return PTR_ERR(chip->irq_sim);
 
Signature · 1 line
2.43.0
Reviewed-by on v2 2/2
#4 lore raw

Re: [PATCH v2 2/2] gpio: sim: lock GPIOs as interrupts when they are requested

Quoted 7 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

Use the extended irq_sim interface to supply the simulated interrupt domain with callbacks allowing the GPIO sim to lock/unlock GPIOs requested as interrupts.

Looks good.

Yours,
Linus Walleij

#5 lore raw
In reply to Bartosz Golaszewski
Quoted 21 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

I realized that the gpio-sim module doesn't lock the GPIOs as interrupts when they are requested from the irq_sim. This leads to users being able to change the direction of GPIOs that should remain as inputs to output. This series extends the irq_sim interface and allows users to supply callbacks that will be executed to inform users about interrupts being requested and released so that they can act accordingly. The gpio-sim is made to use this new API and lock GPIOs as interrupts when needed.

Thomas: if this is fine with you, can you Ack it so that I can take it through the GPIO tree for the next merge window?

Changes since v1:
- drop the notifier in favor of specific callbacks

Bartosz Golaszewski (2):
genirq/irq_sim: add an extended irq_sim initializer
gpio: sim: lock GPIOs as interrupts when they are requested

Quoted 8 lines
 drivers/gpio/gpio-sim.c | 25 ++++++++++++++++-
 include/linux/irq_sim.h | 17 ++++++++++++
 kernel/irq/irq_sim.c    | 60 ++++++++++++++++++++++++++++++++++++++---
 3 files changed, 98 insertions(+), 4 deletions(-)

--
2.43.0

Without any objections, I'd like to take it through the GPIO tree by the end of this week.

Bart

Acked-by on the v2 series
#6 lore raw
In reply to Bartosz Golaszewski
Quoted 10 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

I realized that the gpio-sim module doesn't lock the GPIOs as interrupts when they are requested from the irq_sim. This leads to users being able to change the direction of GPIOs that should remain as inputs to output. This series extends the irq_sim interface and allows users to supply callbacks that will be executed to inform users about interrupts being requested and released so that they can act accordingly. The gpio-sim is made to use this new API and lock GPIOs as interrupts when needed.

Quoted 5 lines

Thomas: if this is fine with you, can you Ack it so that I can take it through the GPIO tree for the next merge window?

Changes since v1:
- drop the notifier in favor of specific callbacks

It sure seems consistent to me and in line with how I once thought about locking lines for IRQ.

Yours,
Linus Walleij

author Applied
#7 lore raw
In reply to Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

Quoted 11 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

I realized that the gpio-sim module doesn't lock the GPIOs as interrupts when they are requested from the irq_sim. This leads to users being able to change the direction of GPIOs that should remain as inputs to output. This series extends the irq_sim interface and allows users to supply callbacks that will be executed to inform users about interrupts being requested and released so that they can act accordingly. The gpio-sim is made to use this new API and lock GPIOs as interrupts when needed.

[...]

Applied, thanks!

[1/2] genirq/irq_sim: add an extended irq_sim initializer

      commit: 011f583781fa46699f1d4c4e9c39ad68f05ced2d
[2/2] gpio: sim: lock GPIOs as interrupts when they are requested
      commit: 9d9c1796a6ae70290c2e013fe4d79e99039a1015

Best regards,

Signature · 1 line
Bartosz Golaszewski <[email protected]>