diff mbox series

[v3,18/24] of: irq: add wake capable bit to of_irq_resource()

Message ID 20231226122113.v3.18.I29b26a7f3b80fac0a618707446a10b6cc974fdaf@changeid
State New
Headers
Series Improve IRQ wake capability reporting and update the cros_ec driver to use it |

Commit Message

Mark Hasemeyer Dec. 26, 2023, 7:21 p.m. UTC 
  Add wake capability information to the IRQ resource. Wake capability is
assumed based on conventions provided in the devicetree wakeup-source
binding documentation. An interrupt is considered wake capable if the
following are true:
1. A wakeup-source property exits in the same device node as the
   interrupt.
2. The IRQ is marked as dedicated by setting its interrupt-name to
   "wakeup".

The wakeup-source documentation states that dedicated interrupts can use
device specific interrupt names and device drivers are still welcome to
use their own naming schemes. This API is provided as a helper if one is
willing to conform to the above conventions.

The ACPI subsystems already provides similar APIs that allow one to
query the wake capability of an IRQ. This brings closer feature parity
to the devicetree.

Signed-off-by: Mark Hasemeyer <markhas@chromium.org>
---

Changes in v3:
-Use DEFINE_RES_IRQ_NAMED_FLAGS macro

Changes in v2:
-Update logic to return true only if wakeup-source property and
 "wakeup" interrupt-name are defined
-irq->IRQ, api->API

 drivers/of/irq.c | 39 +++++++++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)

Comments

Andy Shevchenko Dec. 27, 2023, 5:19 p.m. UTC | #1 
On Tue, Dec 26, 2023 at 12:21:22PM -0700, Mark Hasemeyer wrote:
> Add wake capability information to the IRQ resource. Wake capability is
> assumed based on conventions provided in the devicetree wakeup-source
> binding documentation. An interrupt is considered wake capable if the
> following are true:
> 1. A wakeup-source property exits in the same device node as the
>    interrupt.
> 2. The IRQ is marked as dedicated by setting its interrupt-name to
>    "wakeup".
> 
> The wakeup-source documentation states that dedicated interrupts can use
> device specific interrupt names and device drivers are still welcome to
> use their own naming schemes. This API is provided as a helper if one is
> willing to conform to the above conventions.
> 
> The ACPI subsystems already provides similar APIs that allow one to
> query the wake capability of an IRQ. This brings closer feature parity
> to the devicetree.

...

> +		u32 irq_flags;
>  		const char *name = NULL;

Don't know if OF style requires reversed xmas tree order. If so, this should be

		const char *name = NULL;
		u32 irq_flags;

Otherwise looks good to me, FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Mark Hasemeyer Dec. 27, 2023, 6:21 p.m. UTC | #2 
>
> > +             u32 irq_flags;
> >               const char *name = NULL;
>
> Don't know if OF style requires reversed xmas tree order. If so, this should be
>
>                 const char *name = NULL;
>                 u32 irq_flags;
>

I see both methods used. For example, of_irq_init() uses normal xmas
tree order. I'll leave it unless Rob says otherwise.
It is Christmas time, so I do want to honor the Christmas trees appropriately!
Rob Herring Jan. 2, 2024, 4:01 p.m. UTC | #3 
On Wed, Dec 27, 2023 at 11:21:14AM -0700, Mark Hasemeyer wrote:
> >
> > > +             u32 irq_flags;
> > >               const char *name = NULL;
> >
> > Don't know if OF style requires reversed xmas tree order. If so, this should be
> >
> >                 const char *name = NULL;
> >                 u32 irq_flags;
> >
> 
> I see both methods used. For example, of_irq_init() uses normal xmas
> tree order. I'll leave it unless Rob says otherwise.
> It is Christmas time, so I do want to honor the Christmas trees appropriately!

DT requires sideways Christmas tree because we're special. You'll have 
to add a 3rd variable. ;)

Rob
Rob Herring Jan. 2, 2024, 4:01 p.m. UTC | #4 
On Tue, 26 Dec 2023 12:21:22 -0700, Mark Hasemeyer wrote:
> Add wake capability information to the IRQ resource. Wake capability is
> assumed based on conventions provided in the devicetree wakeup-source
> binding documentation. An interrupt is considered wake capable if the
> following are true:
> 1. A wakeup-source property exits in the same device node as the
>    interrupt.
> 2. The IRQ is marked as dedicated by setting its interrupt-name to
>    "wakeup".
> 
> The wakeup-source documentation states that dedicated interrupts can use
> device specific interrupt names and device drivers are still welcome to
> use their own naming schemes. This API is provided as a helper if one is
> willing to conform to the above conventions.
> 
> The ACPI subsystems already provides similar APIs that allow one to
> query the wake capability of an IRQ. This brings closer feature parity
> to the devicetree.
> 
> Signed-off-by: Mark Hasemeyer <markhas@chromium.org>
> ---
> 
> Changes in v3:
> -Use DEFINE_RES_IRQ_NAMED_FLAGS macro
> 
> Changes in v2:
> -Update logic to return true only if wakeup-source property and
>  "wakeup" interrupt-name are defined
> -irq->IRQ, api->API
> 
>  drivers/of/irq.c | 39 +++++++++++++++++++++++++++++++++++----
>  1 file changed, 35 insertions(+), 4 deletions(-)
> 

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

Patch

diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 174900072c18c..cdecdc3515f88 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -383,11 +383,39 @@  int of_irq_parse_one(struct device_node *device, int index, struct of_phandle_ar
 }
 EXPORT_SYMBOL_GPL(of_irq_parse_one);
 
+/**
+ * __of_irq_wake_capable - Determine whether a given IRQ index is wake capable
+ *
+ * The IRQ is considered wake capable if the following are true:
+ * 1. wakeup-source property exists
+ * 2. provided IRQ index is labelled as a dedicated wakeirq
+ *
+ * This logic assumes the provided IRQ index is valid.
+ *
+ * @dev: pointer to device tree node
+ * @index: zero-based index of the IRQ
+ * Return: True if provided IRQ index for #dev is wake capable. False otherwise.
+ */
+static bool __of_irq_wake_capable(const struct device_node *dev, int index)
+{
+	int wakeindex;
+
+	if (!of_property_read_bool(dev, "wakeup-source"))
+		return false;
+
+	wakeindex = of_property_match_string(dev, "interrupt-names", "wakeup");
+	return wakeindex >= 0 && wakeindex == index;
+}
+
 /**
  * of_irq_to_resource - Decode a node's IRQ and return it as a resource
  * @dev: pointer to device tree node
- * @index: zero-based index of the irq
+ * @index: zero-based index of the IRQ
  * @r: pointer to resource structure to return result into.
+ *
+ * Return: Linux IRQ number on success, or 0 on the IRQ mapping failure, or
+ * -EPROBE_DEFER if the IRQ domain is not yet created, or error code in case
+ * of any other failure.
  */
 int of_irq_to_resource(struct device_node *dev, int index, struct resource *r)
 {
@@ -399,6 +427,7 @@  int of_irq_to_resource(struct device_node *dev, int index, struct resource *r)
 	/* Only dereference the resource if both the
 	 * resource and the irq are valid. */
 	if (r && irq) {
+		u32 irq_flags;
 		const char *name = NULL;
 
 		memset(r, 0, sizeof(*r));
@@ -409,9 +438,11 @@  int of_irq_to_resource(struct device_node *dev, int index, struct resource *r)
 		of_property_read_string_index(dev, "interrupt-names", index,
 					      &name);
 
-		r->start = r->end = irq;
-		r->flags = IORESOURCE_IRQ | irqd_get_trigger_type(irq_get_irq_data(irq));
-		r->name = name ? name : of_node_full_name(dev);
+		irq_flags = irqd_get_trigger_type(irq_get_irq_data(irq));
+		if (__of_irq_wake_capable(dev, index))
+			irq_flags |= IORESOURCE_IRQ_WAKECAPABLE;
+
+		*r = DEFINE_RES_IRQ_NAMED_FLAGS(irq, name ?: of_node_full_name(dev), irq_flags);
 	}
 
 	return irq;