linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 00/24] Improve IRQ wake capability reporting and update the cros_ec driver to use it
@ 2024-01-02 21:07 Mark Hasemeyer
  2024-01-02 21:07 ` [PATCH v4 02/24] gpiolib: acpi: Modify acpi_dev_irq_wake_get_by() to use resource Mark Hasemeyer
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Mark Hasemeyer @ 2024-01-02 21:07 UTC (permalink / raw)
  To: LKML
  Cc: Sudeep Holla, AngeloGioacchino Del Regno, Rob Herring,
	Andy Shevchenko, Krzysztof Kozlowski, Konrad Dybcio, Raul Rangel,
	Tzung-Bi Shih, Mark Hasemeyer, AKASHI Takahiro, Alexandre TORGUE,
	Alim Akhtar, Andre Przywara, Andrew Morton, Andy Shevchenko,
	Baoquan He, Bartosz Golaszewski, Benson Leung,
	Bhanu Prakash Maiya, Bjorn Andersson, Chen-Yu Tsai, Conor Dooley,
	Daniel Scally, David Gow, Frank Rowand, Greg Kroah-Hartman,
	Guenter Roeck, Heikki Krogerus, Heiko Stuebner, Jonathan Hunter,
	Krzysztof Kozlowski, Len Brown, Linus Walleij, Mark Brown,
	Matthias Brugger, Mika Westerberg, Nick Hawkins, Paul Barker,
	Prashant Malani, Rafael J. Wysocki, Rob Barnes, Rob Herring,
	Romain Perier, Sakari Ailus, Stephen Boyd, Takashi Iwai,
	Thierry Reding, Uwe Kleine-König, Wei Xu, Wolfram Sang,
	chrome-platform, cros-qcom-dts-watchers, devicetree, linux-acpi,
	linux-arm-kernel, linux-arm-msm, linux-gpio, linux-i2c,
	linux-mediatek, linux-rockchip, linux-samsung-soc, linux-tegra

Currently the cros_ec driver assumes that its associated interrupt is
wake capable. This is an incorrect assumption as some Chromebooks use a
separate wake pin, while others overload the interrupt for wake and IO.
This patch train updates the driver to query the underlying ACPI/DT data
to determine whether or not the IRQ should be enabled for wake.

Both the device tree and ACPI systems have methods for reporting IRQ
wake capability. In device tree based systems, a node can advertise
itself as a 'wakeup-source'. In ACPI based systems, GpioInt and
Interrupt resource descriptors can use the 'SharedAndWake' or
'ExclusiveAndWake' share types.

Some logic is added to the platform, ACPI, and DT subsystems to more
easily pipe wakeirq information up to the driver.

Changes in v4:
-Rebase on linux-next
-See each patch for patch specific changes

Changes in v3:
-Rebase on linux-next
-See each patch for patch specific changes

Changes in v2:
-Rebase on linux-next
-Add cover letter
-See each patch for patch specific changes

Mark Hasemeyer (24):
  resource: Add DEFINE_RES_*_NAMED_FLAGS macro
  gpiolib: acpi: Modify acpi_dev_irq_wake_get_by() to use resource
  i2c: acpi: Modify i2c_acpi_get_irq() to use resource
  dt-bindings: power: Clarify wording for wakeup-source property
  ARM: dts: tegra: Enable cros-ec-spi as wake source
  ARM: dts: rockchip: rk3288: Enable cros-ec-spi as wake source
  ARM: dts: samsung: exynos5420: Enable cros-ec-spi as wake source
  ARM: dts: samsung: exynos5800: Enable cros-ec-spi as wake source
  arm64: dts: mediatek: mt8173: Enable cros-ec-spi as wake source
  arm64: dts: mediatek: mt8183: Enable cros-ec-spi as wake source
  arm64: dts: mediatek: mt8192: Enable cros-ec-spi as wake source
  arm64: dts: mediatek: mt8195: Enable cros-ec-spi as wake source
  arm64: dts: tegra: Enable cros-ec-spi as wake source
  arm64: dts: qcom: sc7180: Enable cros-ec-spi as wake source
  arm64: dts: qcom: sc7280: Enable cros-ec-spi as wake source
  arm64: dts: qcom: sdm845: Enable cros-ec-spi as wake source
  arm64: dts: rockchip: rk3399: Enable cros-ec-spi as wake source
  of: irq: add wake capable bit to of_irq_resource()
  of: irq: Add default implementation for of_irq_to_resource()
  of: irq: Remove extern from function declarations
  device property: Modify fwnode irq_get() to use resource
  device property: Update functions to use EXPORT_SYMBOL_GPL()
  platform: Modify platform_get_irq_optional() to use resource
  platform/chrome: cros_ec: Use PM subsystem to manage wakeirq

 .../bindings/power/wakeup-source.txt          | 18 ++--
 arch/arm/boot/dts/nvidia/tegra124-nyan.dtsi   |  1 +
 arch/arm/boot/dts/nvidia/tegra124-venice2.dts |  1 +
 .../rockchip/rk3288-veyron-chromebook.dtsi    |  1 +
 .../boot/dts/samsung/exynos5420-peach-pit.dts |  1 +
 .../boot/dts/samsung/exynos5800-peach-pi.dts  |  1 +
 arch/arm64/boot/dts/mediatek/mt8173-elm.dtsi  |  1 +
 .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi |  1 +
 .../boot/dts/mediatek/mt8192-asurada.dtsi     |  1 +
 .../boot/dts/mediatek/mt8195-cherry.dtsi      |  1 +
 .../arm64/boot/dts/nvidia/tegra132-norrin.dts |  1 +
 arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi  |  1 +
 .../arm64/boot/dts/qcom/sc7280-herobrine.dtsi |  1 +
 .../arm64/boot/dts/qcom/sc7280-idp-ec-h1.dtsi |  1 +
 arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi    |  1 +
 arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi  |  1 +
 drivers/acpi/property.c                       | 11 ++-
 drivers/base/platform.c                       | 90 ++++++++++++-------
 drivers/base/property.c                       | 40 ++++++---
 drivers/gpio/gpiolib-acpi.c                   | 28 ++++--
 drivers/i2c/i2c-core-acpi.c                   | 43 ++++-----
 drivers/i2c/i2c-core-base.c                   |  6 +-
 drivers/i2c/i2c-core.h                        |  4 +-
 drivers/of/irq.c                              | 39 +++++++-
 drivers/of/property.c                         |  8 +-
 drivers/platform/chrome/cros_ec.c             | 48 ++++++++--
 drivers/platform/chrome/cros_ec_lpc.c         | 40 +++++++--
 drivers/platform/chrome/cros_ec_spi.c         | 15 ++--
 drivers/platform/chrome/cros_ec_uart.c        | 14 ++-
 include/linux/acpi.h                          | 25 +++---
 include/linux/fwnode.h                        |  8 +-
 include/linux/ioport.h                        | 20 +++--
 include/linux/of_irq.h                        | 41 +++++----
 include/linux/platform_data/cros_ec_proto.h   |  4 +-
 include/linux/platform_device.h               |  3 +
 include/linux/property.h                      |  2 +
 36 files changed, 350 insertions(+), 172 deletions(-)

-- 
2.43.0.472.g3155946c3a-goog


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

* [PATCH v4 02/24] gpiolib: acpi: Modify acpi_dev_irq_wake_get_by() to use resource
  2024-01-02 21:07 [PATCH v4 00/24] Improve IRQ wake capability reporting and update the cros_ec driver to use it Mark Hasemeyer
@ 2024-01-02 21:07 ` Mark Hasemeyer
  2024-01-06 14:44   ` Andy Shevchenko
  2024-01-02 21:07 ` [PATCH v4 03/24] i2c: acpi: Modify i2c_acpi_get_irq() " Mark Hasemeyer
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Mark Hasemeyer @ 2024-01-02 21:07 UTC (permalink / raw)
  To: LKML
  Cc: Sudeep Holla, AngeloGioacchino Del Regno, Rob Herring,
	Andy Shevchenko, Krzysztof Kozlowski, Konrad Dybcio, Raul Rangel,
	Tzung-Bi Shih, Mark Hasemeyer, Andy Shevchenko,
	Bartosz Golaszewski, Len Brown, Linus Walleij, Mika Westerberg,
	Rafael J. Wysocki, Wolfram Sang, linux-acpi, linux-gpio,
	linux-i2c

Other information besides wake capability can be provided about GPIO
IRQs such as triggering, polarity, and sharability. Use resource flags
to provide this information to the caller if they want it.

This should keep the API more robust over time as flags are added,
modified, or removed. It also more closely matches acpi_irq_get() which
take a resource as an argument.

Rename the function to acpi_dev_get_gpio_irq_resource() to better
describe the function's new behavior.
Signed-off-by: Mark Hasemeyer <markhas@chromium.org>
---

Changes in v4:
-DEFINES_RES_NAMED->DEFINE_RES_IRQ_NAMED_FLAGS
-Indent fix
-Initialize struct resource on stack
-Remove ioport.h dependency in acpi.h

Changes in v3:
-Use DEFINE_RES_NAMED macro
-Add acpi_gpio_info.shareable doc

Changes in v2:
-Remove explicit cast to struct resource
-irq -> IRQ

 drivers/gpio/gpiolib-acpi.c | 28 +++++++++++++++++++---------
 drivers/i2c/i2c-core-acpi.c | 10 ++++++++--
 include/linux/acpi.h        | 25 +++++++++++--------------
 3 files changed, 38 insertions(+), 25 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 88066826d8e5b..d14426c831187 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -12,6 +12,7 @@
 #include <linux/errno.h>
 #include <linux/export.h>
 #include <linux/interrupt.h>
+#include <linux/ioport.h>
 #include <linux/irq.h>
 #include <linux/mutex.h>
 #include <linux/pinctrl/pinctrl.h>
@@ -99,6 +100,7 @@ struct acpi_gpio_chip {
  * @pin_config: pin bias as provided by ACPI
  * @polarity: interrupt polarity as provided by ACPI
  * @triggering: triggering type as provided by ACPI
+ * @shareable: share type as provided by ACPI (shared vs exclusive).
  * @wake_capable: wake capability as provided by ACPI
  * @debounce: debounce timeout as provided by ACPI
  * @quirks: Linux specific quirks as provided by struct acpi_gpio_mapping
@@ -111,6 +113,7 @@ struct acpi_gpio_info {
 	int polarity;
 	int triggering;
 	bool wake_capable;
+	bool shareable;
 	unsigned int debounce;
 	unsigned int quirks;
 };
@@ -760,6 +763,7 @@ static int acpi_populate_gpio_lookup(struct acpi_resource *ares, void *data)
 		lookup->info.debounce = agpio->debounce_timeout;
 		lookup->info.gpioint = gpioint;
 		lookup->info.wake_capable = acpi_gpio_irq_is_wake(&lookup->info.adev->dev, agpio);
+		lookup->info.shareable = agpio->shareable == ACPI_SHARED;
 
 		/*
 		 * Polarity and triggering are only specified for GpioInt
@@ -1004,11 +1008,11 @@ struct gpio_desc *acpi_find_gpio(struct fwnode_handle *fwnode,
 }
 
 /**
- * acpi_dev_gpio_irq_wake_get_by() - Find GpioInt and translate it to Linux IRQ number
+ * acpi_dev_get_gpio_irq_resource() - Find GpioInt and populate resource struct
  * @adev: pointer to a ACPI device to get IRQ from
  * @name: optional name of GpioInt resource
  * @index: index of GpioInt resource (starting from %0)
- * @wake_capable: Set to true if the IRQ is wake capable
+ * @r: pointer to resource to populate with IRQ information.
  *
  * If the device has one or more GpioInt resources, this function can be
  * used to translate from the GPIO offset in the resource to the Linux IRQ
@@ -1023,10 +1027,12 @@ struct gpio_desc *acpi_find_gpio(struct fwnode_handle *fwnode,
  * The GPIO is considered wake capable if the GpioInt resource specifies
  * SharedAndWake or ExclusiveAndWake.
  *
- * Return: Linux IRQ number (> %0) on success, negative errno on failure.
+ * IRQ number will be available in the resource structure.
+ *
+ * Return: 0 on success, negative errno on failure.
  */
-int acpi_dev_gpio_irq_wake_get_by(struct acpi_device *adev, const char *name, int index,
-				  bool *wake_capable)
+int acpi_dev_get_gpio_irq_resource(struct acpi_device *adev, const char *name, int index,
+				   struct resource *r)
 {
 	int idx, i;
 	unsigned int irq_flags;
@@ -1045,6 +1051,7 @@ int acpi_dev_gpio_irq_wake_get_by(struct acpi_device *adev, const char *name, in
 		if (info.gpioint && idx++ == index) {
 			unsigned long lflags = GPIO_LOOKUP_FLAGS_DEFAULT;
 			enum gpiod_flags dflags = GPIOD_ASIS;
+			unsigned long res_flags;
 			char label[32];
 			int irq;
 
@@ -1084,16 +1091,19 @@ int acpi_dev_gpio_irq_wake_get_by(struct acpi_device *adev, const char *name, in
 			}
 
 			/* avoid suspend issues with GPIOs when systems are using S3 */
-			if (wake_capable && acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0)
-				*wake_capable = info.wake_capable;
+			if (info.wake_capable && !(acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0))
+				info.wake_capable = false;
 
-			return irq;
+			res_flags = acpi_dev_irq_flags(info.triggering, info.polarity,
+						       info.shareable, info.wake_capable);
+			*r = DEFINE_RES_IRQ_NAMED_FLAGS(irq, NULL, res_flags);
+			return 0;
 		}
 
 	}
 	return -ENOENT;
 }
-EXPORT_SYMBOL_GPL(acpi_dev_gpio_irq_wake_get_by);
+EXPORT_SYMBOL_GPL(acpi_dev_get_gpio_irq_resource);
 
 static acpi_status
 acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c
index d6037a3286690..8126a87baf3d4 100644
--- a/drivers/i2c/i2c-core-acpi.c
+++ b/drivers/i2c/i2c-core-acpi.c
@@ -203,6 +203,7 @@ int i2c_acpi_get_irq(struct i2c_client *client, bool *wake_capable)
 {
 	struct acpi_device *adev = ACPI_COMPANION(&client->dev);
 	struct list_head resource_list;
+	struct resource irqres;
 	struct i2c_acpi_irq_context irq_ctx = {
 		.irq = -ENOENT,
 	};
@@ -217,8 +218,13 @@ int i2c_acpi_get_irq(struct i2c_client *client, bool *wake_capable)
 
 	acpi_dev_free_resource_list(&resource_list);
 
-	if (irq_ctx.irq == -ENOENT)
-		irq_ctx.irq = acpi_dev_gpio_irq_wake_get(adev, 0, &irq_ctx.wake_capable);
+	if (irq_ctx.irq == -ENOENT) {
+		ret = acpi_dev_get_gpio_irq_resource(adev, NULL, 0, &irqres);
+		if (ret)
+			return ret;
+		irq_ctx.irq = irqres.start;
+		irq_ctx.wake_capable = irqres.flags & IORESOURCE_IRQ_WAKECAPABLE;
+	}
 
 	if (irq_ctx.irq < 0)
 		return irq_ctx.irq;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index b7165e52b3c68..a0cd733febe34 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -9,7 +9,6 @@
 #define _LINUX_ACPI_H
 
 #include <linux/errno.h>
-#include <linux/ioport.h>	/* for struct resource */
 #include <linux/resource_ext.h>
 #include <linux/device.h>
 #include <linux/mod_devicetable.h>
@@ -17,6 +16,7 @@
 #include <linux/uuid.h>
 #include <linux/node.h>
 
+struct resource;
 struct irq_domain;
 struct irq_domain_ops;
 
@@ -1232,8 +1232,8 @@ bool acpi_gpio_get_irq_resource(struct acpi_resource *ares,
 				struct acpi_resource_gpio **agpio);
 bool acpi_gpio_get_io_resource(struct acpi_resource *ares,
 			       struct acpi_resource_gpio **agpio);
-int acpi_dev_gpio_irq_wake_get_by(struct acpi_device *adev, const char *name, int index,
-				  bool *wake_capable);
+int acpi_dev_get_gpio_irq_resource(struct acpi_device *adev, const char *name, int index,
+				   struct resource *r);
 #else
 static inline bool acpi_gpio_get_irq_resource(struct acpi_resource *ares,
 					      struct acpi_resource_gpio **agpio)
@@ -1245,28 +1245,25 @@ static inline bool acpi_gpio_get_io_resource(struct acpi_resource *ares,
 {
 	return false;
 }
-static inline int acpi_dev_gpio_irq_wake_get_by(struct acpi_device *adev, const char *name,
-						int index, bool *wake_capable)
+static inline int acpi_dev_get_gpio_irq_resource(struct acpi_device *adev, const char *name,
+						 int index, struct resource *r)
 {
 	return -ENXIO;
 }
 #endif
 
-static inline int acpi_dev_gpio_irq_wake_get(struct acpi_device *adev, int index,
-					     bool *wake_capable)
+static inline int acpi_dev_gpio_irq_get_by(struct acpi_device *adev, const char *name, int index)
 {
-	return acpi_dev_gpio_irq_wake_get_by(adev, NULL, index, wake_capable);
-}
+	struct resource r = {};
+	int ret;
 
-static inline int acpi_dev_gpio_irq_get_by(struct acpi_device *adev, const char *name,
-					   int index)
-{
-	return acpi_dev_gpio_irq_wake_get_by(adev, name, index, NULL);
+	ret = acpi_dev_get_gpio_irq_resource(adev, name, index, &r);
+	return ret ?: r.start;
 }
 
 static inline int acpi_dev_gpio_irq_get(struct acpi_device *adev, int index)
 {
-	return acpi_dev_gpio_irq_wake_get_by(adev, NULL, index, NULL);
+	return acpi_dev_gpio_irq_get_by(adev, NULL, index);
 }
 
 /* Device properties */
-- 
2.43.0.472.g3155946c3a-goog


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

* [PATCH v4 03/24] i2c: acpi: Modify i2c_acpi_get_irq() to use resource
  2024-01-02 21:07 [PATCH v4 00/24] Improve IRQ wake capability reporting and update the cros_ec driver to use it Mark Hasemeyer
  2024-01-02 21:07 ` [PATCH v4 02/24] gpiolib: acpi: Modify acpi_dev_irq_wake_get_by() to use resource Mark Hasemeyer
@ 2024-01-02 21:07 ` Mark Hasemeyer
  2024-01-06 14:48   ` Andy Shevchenko
  2024-01-02 21:07 ` [PATCH v4 21/24] device property: Modify fwnode irq_get() " Mark Hasemeyer
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Mark Hasemeyer @ 2024-01-02 21:07 UTC (permalink / raw)
  To: LKML
  Cc: Sudeep Holla, AngeloGioacchino Del Regno, Rob Herring,
	Andy Shevchenko, Krzysztof Kozlowski, Konrad Dybcio, Raul Rangel,
	Tzung-Bi Shih, Mark Hasemeyer, Andy Shevchenko, Mika Westerberg,
	Wolfram Sang, linux-acpi, linux-i2c

The i2c_acpi_irq_context structure provides redundant information that
can be provided with struct resource.

Refactor i2c_acpi_get_irq() to use struct resource instead of struct
i2c_acpi_irq_context.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Mark Hasemeyer <markhas@chromium.org>
---

Changes in v4:
-Use Andy's @linux.intel.com email
-Remove blank line in commit message
-More error handling refactoring in i2c_acpi_get_irq()
-Remove struct i2c_acpi_irq_context as it's unused

Changes in v3:
-Add Suggested-by
-Check resource flags for valid irq
-Drop error pointer check
-Invert error checking logic in i2c_acpi_get_irq()
-Drop redundant 0 in struct resource init
-Drop unnecessary check for irq > 0 when setting I2C_CLIENT_WAKE

Changes in v2:
-New patch

 drivers/i2c/i2c-core-acpi.c | 49 +++++++++++++------------------------
 drivers/i2c/i2c-core-base.c |  6 ++---
 drivers/i2c/i2c-core.h      |  4 +--
 3 files changed, 22 insertions(+), 37 deletions(-)

diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c
index 8126a87baf3d4..4c3df540c2f4b 100644
--- a/drivers/i2c/i2c-core-acpi.c
+++ b/drivers/i2c/i2c-core-acpi.c
@@ -137,11 +137,6 @@ static const struct acpi_device_id i2c_acpi_ignored_device_ids[] = {
 	{}
 };
 
-struct i2c_acpi_irq_context {
-	int irq;
-	bool wake_capable;
-};
-
 static int i2c_acpi_do_lookup(struct acpi_device *adev,
 			      struct i2c_acpi_lookup *lookup)
 {
@@ -175,64 +170,54 @@ static int i2c_acpi_do_lookup(struct acpi_device *adev,
 
 static int i2c_acpi_add_irq_resource(struct acpi_resource *ares, void *data)
 {
-	struct i2c_acpi_irq_context *irq_ctx = data;
-	struct resource r;
+	struct resource *r = data;
 
-	if (irq_ctx->irq > 0)
+	if (r->flags)
 		return 1;
 
-	if (!acpi_dev_resource_interrupt(ares, 0, &r))
+	if (!acpi_dev_resource_interrupt(ares, 0, r))
 		return 1;
 
-	irq_ctx->irq = i2c_dev_irq_from_resources(&r, 1);
-	irq_ctx->wake_capable = r.flags & IORESOURCE_IRQ_WAKECAPABLE;
+	i2c_dev_irq_from_resources(r, 1);
 
 	return 1; /* No need to add resource to the list */
 }
 
 /**
- * i2c_acpi_get_irq - get device IRQ number from ACPI
+ * i2c_acpi_get_irq - get device IRQ number from ACPI and populate resource
  * @client: Pointer to the I2C client device
- * @wake_capable: Set to true if the IRQ is wake capable
+ * @r: resource with populated IRQ information
  *
  * Find the IRQ number used by a specific client device.
  *
  * Return: The IRQ number or an error code.
  */
-int i2c_acpi_get_irq(struct i2c_client *client, bool *wake_capable)
+int i2c_acpi_get_irq(struct i2c_client *client, struct resource *r)
 {
 	struct acpi_device *adev = ACPI_COMPANION(&client->dev);
 	struct list_head resource_list;
-	struct resource irqres;
-	struct i2c_acpi_irq_context irq_ctx = {
-		.irq = -ENOENT,
-	};
 	int ret;
 
+	if (!r)
+		return -EINVAL;
+
 	INIT_LIST_HEAD(&resource_list);
 
 	ret = acpi_dev_get_resources(adev, &resource_list,
-				     i2c_acpi_add_irq_resource, &irq_ctx);
+				     i2c_acpi_add_irq_resource, r);
 	if (ret < 0)
 		return ret;
 
 	acpi_dev_free_resource_list(&resource_list);
 
-	if (irq_ctx.irq == -ENOENT) {
-		ret = acpi_dev_get_gpio_irq_resource(adev, NULL, 0, &irqres);
-		if (ret)
-			return ret;
-		irq_ctx.irq = irqres.start;
-		irq_ctx.wake_capable = irqres.flags & IORESOURCE_IRQ_WAKECAPABLE;
-	}
+	if (r->flags)
+		return r->start;
 
-	if (irq_ctx.irq < 0)
-		return irq_ctx.irq;
+	ret = acpi_dev_get_gpio_irq_resource(adev, NULL, 0, r);
+	if (ret)
+		return ret;
 
-	if (wake_capable)
-		*wake_capable = irq_ctx.wake_capable;
-
-	return irq_ctx.irq;
+	return r->start;
 }
 
 static int i2c_acpi_get_info(struct acpi_device *adev,
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 3bd48d4b6318f..0339c298ba50b 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -513,10 +513,10 @@ static int i2c_device_probe(struct device *dev)
 			if (irq == -EINVAL || irq == -ENODATA)
 				irq = of_irq_get(dev->of_node, 0);
 		} else if (ACPI_COMPANION(dev)) {
-			bool wake_capable;
+			struct resource r = {};
 
-			irq = i2c_acpi_get_irq(client, &wake_capable);
-			if (irq > 0 && wake_capable)
+			irq = i2c_acpi_get_irq(client, &r);
+			if (r.flags & IORESOURCE_IRQ_WAKECAPABLE)
 				client->flags |= I2C_CLIENT_WAKE;
 		}
 		if (irq == -EPROBE_DEFER) {
diff --git a/drivers/i2c/i2c-core.h b/drivers/i2c/i2c-core.h
index 05b8b8dfa9bdd..b5dc559c49d11 100644
--- a/drivers/i2c/i2c-core.h
+++ b/drivers/i2c/i2c-core.h
@@ -61,11 +61,11 @@ static inline int __i2c_check_suspended(struct i2c_adapter *adap)
 #ifdef CONFIG_ACPI
 void i2c_acpi_register_devices(struct i2c_adapter *adap);
 
-int i2c_acpi_get_irq(struct i2c_client *client, bool *wake_capable);
+int i2c_acpi_get_irq(struct i2c_client *client, struct resource *r);
 #else /* CONFIG_ACPI */
 static inline void i2c_acpi_register_devices(struct i2c_adapter *adap) { }
 
-static inline int i2c_acpi_get_irq(struct i2c_client *client, bool *wake_capable)
+static inline int i2c_acpi_get_irq(struct i2c_client *client, struct resource *r)
 {
 	return 0;
 }
-- 
2.43.0.472.g3155946c3a-goog


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

* [PATCH v4 21/24] device property: Modify fwnode irq_get() to use resource
  2024-01-02 21:07 [PATCH v4 00/24] Improve IRQ wake capability reporting and update the cros_ec driver to use it Mark Hasemeyer
  2024-01-02 21:07 ` [PATCH v4 02/24] gpiolib: acpi: Modify acpi_dev_irq_wake_get_by() to use resource Mark Hasemeyer
  2024-01-02 21:07 ` [PATCH v4 03/24] i2c: acpi: Modify i2c_acpi_get_irq() " Mark Hasemeyer
@ 2024-01-02 21:07 ` Mark Hasemeyer
  2024-01-06 14:52   ` Andy Shevchenko
  2024-01-09 20:47   ` Rob Herring
  2024-01-02 21:07 ` [PATCH v4 22/24] device property: Update functions to use EXPORT_SYMBOL_GPL() Mark Hasemeyer
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 13+ messages in thread
From: Mark Hasemeyer @ 2024-01-02 21:07 UTC (permalink / raw)
  To: LKML
  Cc: Sudeep Holla, AngeloGioacchino Del Regno, Rob Herring,
	Andy Shevchenko, Krzysztof Kozlowski, Konrad Dybcio, Raul Rangel,
	Tzung-Bi Shih, Mark Hasemeyer, Andy Shevchenko, Sakari Ailus,
	Daniel Scally, Frank Rowand, Greg Kroah-Hartman, Heikki Krogerus,
	Len Brown, Rafael J. Wysocki, Rob Herring, devicetree,
	linux-acpi

The underlying ACPI and OF subsystems provide their own APIs which
provide IRQ information as a struct resource. This allows callers to get
more information about the IRQ by looking at the resource flags. For
example, whether or not an IRQ is wake capable.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mark Hasemeyer <markhas@chromium.org>
---

Changes in v4:
-Add Sakari's Reviewed-by tag from v2
-Remove ioport.h dependency in fwnode.h
-Use Andy's @linux.intel.com email

Changes in v3:
-Add Suggested-by tag
-Initialize struct resource to 0 on stack
-EXPORT_SYMBOL()->EXPORT_SYMBOL_GPL()
-Remove extra space in commit message
-Reformat fwnode_irq_get_resource() declaration

Changes in v2:
-New patch

 drivers/acpi/property.c  | 11 +++++------
 drivers/base/property.c  | 32 +++++++++++++++++++++++++-------
 drivers/of/property.c    |  8 ++++----
 include/linux/fwnode.h   |  8 +++++---
 include/linux/property.h |  2 ++
 5 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index a6ead5204046b..891fff5a16797 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -1627,17 +1627,16 @@ static int acpi_fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,
 	return 0;
 }
 
-static int acpi_fwnode_irq_get(const struct fwnode_handle *fwnode,
-			       unsigned int index)
+static int acpi_fwnode_irq_get_resource(const struct fwnode_handle *fwnode, unsigned int index,
+					struct resource *r)
 {
-	struct resource res;
 	int ret;
 
-	ret = acpi_irq_get(ACPI_HANDLE_FWNODE(fwnode), index, &res);
+	ret = acpi_irq_get(ACPI_HANDLE_FWNODE(fwnode), index, r);
 	if (ret)
 		return ret;
 
-	return res.start;
+	return r->start;
 }
 
 #define DECLARE_ACPI_FWNODE_OPS(ops) \
@@ -1664,7 +1663,7 @@ static int acpi_fwnode_irq_get(const struct fwnode_handle *fwnode,
 			acpi_graph_get_remote_endpoint,			\
 		.graph_get_port_parent = acpi_fwnode_get_parent,	\
 		.graph_parse_endpoint = acpi_fwnode_graph_parse_endpoint, \
-		.irq_get = acpi_fwnode_irq_get,				\
+		.irq_get_resource = acpi_fwnode_irq_get_resource,	\
 	};								\
 	EXPORT_SYMBOL_GPL(ops)
 
diff --git a/drivers/base/property.c b/drivers/base/property.c
index a1b01ab420528..441899171d19d 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1046,6 +1046,29 @@ void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int index)
 }
 EXPORT_SYMBOL(fwnode_iomap);
 
+/**
+ * fwnode_irq_get_resource - Get IRQ directly from a fwnode and populate
+ *			     the resource struct
+ * @fwnode:	Pointer to the firmware node
+ * @index:	Zero-based index of the IRQ
+ * @r:		Pointer to resource to populate with IRQ information.
+ *
+ * Return: Linux IRQ number on success. Negative errno on failure.
+ */
+int fwnode_irq_get_resource(const struct fwnode_handle *fwnode, unsigned int index,
+			    struct resource *r)
+{
+	int ret;
+
+	ret = fwnode_call_int_op(fwnode, irq_get_resource, index, r);
+	/* We treat mapping errors as invalid case */
+	if (ret == 0)
+		return -EINVAL;
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(fwnode_irq_get_resource);
+
 /**
  * fwnode_irq_get - Get IRQ directly from a fwnode
  * @fwnode:	Pointer to the firmware node
@@ -1055,14 +1078,9 @@ EXPORT_SYMBOL(fwnode_iomap);
  */
 int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index)
 {
-	int ret;
+	struct resource r = {};
 
-	ret = fwnode_call_int_op(fwnode, irq_get, index);
-	/* We treat mapping errors as invalid case */
-	if (ret == 0)
-		return -EINVAL;
-
-	return ret;
+	return fwnode_irq_get_resource(fwnode, index, &r);
 }
 EXPORT_SYMBOL(fwnode_irq_get);
 
diff --git a/drivers/of/property.c b/drivers/of/property.c
index afdaefbd03f61..864ea5fa5702b 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -1425,10 +1425,10 @@ static void __iomem *of_fwnode_iomap(struct fwnode_handle *fwnode, int index)
 #endif
 }
 
-static int of_fwnode_irq_get(const struct fwnode_handle *fwnode,
-			     unsigned int index)
+static int of_fwnode_irq_get_resource(const struct fwnode_handle *fwnode,
+				      unsigned int index, struct resource *r)
 {
-	return of_irq_get(to_of_node(fwnode), index);
+	return of_irq_to_resource(to_of_node(fwnode), index, r);
 }
 
 static int of_fwnode_add_links(struct fwnode_handle *fwnode)
@@ -1469,7 +1469,7 @@ const struct fwnode_operations of_fwnode_ops = {
 	.graph_get_port_parent = of_fwnode_graph_get_port_parent,
 	.graph_parse_endpoint = of_fwnode_graph_parse_endpoint,
 	.iomap = of_fwnode_iomap,
-	.irq_get = of_fwnode_irq_get,
+	.irq_get_resource = of_fwnode_irq_get_resource,
 	.add_links = of_fwnode_add_links,
 };
 EXPORT_SYMBOL_GPL(of_fwnode_ops);
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index 2a72f55d26eb8..b82c9c072bcc9 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -9,12 +9,13 @@
 #ifndef _LINUX_FWNODE_H_
 #define _LINUX_FWNODE_H_
 
-#include <linux/types.h>
-#include <linux/list.h>
 #include <linux/bits.h>
 #include <linux/err.h>
+#include <linux/list.h>
+#include <linux/types.h>
 
 struct fwnode_operations;
+struct resource;
 struct device;
 
 /*
@@ -164,7 +165,8 @@ struct fwnode_operations {
 	int (*graph_parse_endpoint)(const struct fwnode_handle *fwnode,
 				    struct fwnode_endpoint *endpoint);
 	void __iomem *(*iomap)(struct fwnode_handle *fwnode, int index);
-	int (*irq_get)(const struct fwnode_handle *fwnode, unsigned int index);
+	int (*irq_get_resource)(const struct fwnode_handle *fwnode,
+				unsigned int index, struct resource *r);
 	int (*add_links)(struct fwnode_handle *fwnode);
 };
 
diff --git a/include/linux/property.h b/include/linux/property.h
index e6516d0b7d52a..685ba72a8ce9e 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -190,6 +190,8 @@ struct fwnode_handle *fwnode_handle_get(struct fwnode_handle *fwnode);
 void fwnode_handle_put(struct fwnode_handle *fwnode);
 
 int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index);
+int fwnode_irq_get_resource(const struct fwnode_handle *fwnode,
+			    unsigned int index, struct resource *r);
 int fwnode_irq_get_byname(const struct fwnode_handle *fwnode, const char *name);
 
 unsigned int device_get_child_node_count(const struct device *dev);
-- 
2.43.0.472.g3155946c3a-goog


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

* [PATCH v4 22/24] device property: Update functions to use EXPORT_SYMBOL_GPL()
  2024-01-02 21:07 [PATCH v4 00/24] Improve IRQ wake capability reporting and update the cros_ec driver to use it Mark Hasemeyer
                   ` (2 preceding siblings ...)
  2024-01-02 21:07 ` [PATCH v4 21/24] device property: Modify fwnode irq_get() " Mark Hasemeyer
@ 2024-01-02 21:07 ` Mark Hasemeyer
  2024-01-08 18:32   ` Sakari Ailus
  2024-01-22  9:14 ` [PATCH v4 00/24] Improve IRQ wake capability reporting and update the cros_ec driver to use it Matthias Brugger
  2024-02-14 17:57 ` (subset) " Bjorn Andersson
  5 siblings, 1 reply; 13+ messages in thread
From: Mark Hasemeyer @ 2024-01-02 21:07 UTC (permalink / raw)
  To: LKML
  Cc: Sudeep Holla, AngeloGioacchino Del Regno, Rob Herring,
	Andy Shevchenko, Krzysztof Kozlowski, Konrad Dybcio, Raul Rangel,
	Tzung-Bi Shih, Mark Hasemeyer, Sakari Ailus, Andy Shevchenko,
	Daniel Scally, Greg Kroah-Hartman, Heikki Krogerus,
	Rafael J. Wysocki, linux-acpi

Some of the exported functions use EXPORT_SYMBOL() instead of
EXPORT_SYMBOL_GPL() and are inconsistent with the other exported
functions in the module. The underlying APCI/OF struct fwnode_operations
implementations are also exported via EXPORT_SYMBOL_GPL().

Update them to use the EXPORT_SYMBOL_GPL() macro.

Suggested-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Mark Hasemeyer <markhas@chromium.org>
---

Changes in v4:
-EXPORT_SYMBOL->EXPORT_SYMBOL()
-Add Andy's Reviewed-by tag

Changes in v3:
-New patch

 drivers/base/property.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 441899171d19d..4f686516cac82 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1044,7 +1044,7 @@ void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int index)
 {
 	return fwnode_call_ptr_op(fwnode, iomap, index);
 }
-EXPORT_SYMBOL(fwnode_iomap);
+EXPORT_SYMBOL_GPL(fwnode_iomap);
 
 /**
  * fwnode_irq_get_resource - Get IRQ directly from a fwnode and populate
@@ -1082,7 +1082,7 @@ int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index)
 
 	return fwnode_irq_get_resource(fwnode, index, &r);
 }
-EXPORT_SYMBOL(fwnode_irq_get);
+EXPORT_SYMBOL_GPL(fwnode_irq_get);
 
 /**
  * fwnode_irq_get_byname - Get IRQ from a fwnode using its name
@@ -1110,7 +1110,7 @@ int fwnode_irq_get_byname(const struct fwnode_handle *fwnode, const char *name)
 
 	return fwnode_irq_get(fwnode, index);
 }
-EXPORT_SYMBOL(fwnode_irq_get_byname);
+EXPORT_SYMBOL_GPL(fwnode_irq_get_byname);
 
 /**
  * fwnode_graph_get_next_endpoint - Get next endpoint firmware node
@@ -1355,7 +1355,7 @@ int fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,
 
 	return fwnode_call_int_op(fwnode, graph_parse_endpoint, endpoint);
 }
-EXPORT_SYMBOL(fwnode_graph_parse_endpoint);
+EXPORT_SYMBOL_GPL(fwnode_graph_parse_endpoint);
 
 const void *device_get_match_data(const struct device *dev)
 {
-- 
2.43.0.472.g3155946c3a-goog


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

* Re: [PATCH v4 02/24] gpiolib: acpi: Modify acpi_dev_irq_wake_get_by() to use resource
  2024-01-02 21:07 ` [PATCH v4 02/24] gpiolib: acpi: Modify acpi_dev_irq_wake_get_by() to use resource Mark Hasemeyer
@ 2024-01-06 14:44   ` Andy Shevchenko
  2024-01-08 19:08     ` Mark Hasemeyer
  0 siblings, 1 reply; 13+ messages in thread
From: Andy Shevchenko @ 2024-01-06 14:44 UTC (permalink / raw)
  To: Mark Hasemeyer
  Cc: LKML, Sudeep Holla, AngeloGioacchino Del Regno, Rob Herring,
	Krzysztof Kozlowski, Konrad Dybcio, Raul Rangel, Tzung-Bi Shih,
	Bartosz Golaszewski, Len Brown, Linus Walleij, Mika Westerberg,
	Rafael J. Wysocki, Wolfram Sang, linux-acpi, linux-gpio,
	linux-i2c

On Tue, Jan 02, 2024 at 02:07:26PM -0700, Mark Hasemeyer wrote:
> Other information besides wake capability can be provided about GPIO
> IRQs such as triggering, polarity, and sharability. Use resource flags
> to provide this information to the caller if they want it.
> 
> This should keep the API more robust over time as flags are added,
> modified, or removed. It also more closely matches acpi_irq_get() which
> take a resource as an argument.
> 
> Rename the function to acpi_dev_get_gpio_irq_resource() to better
> describe the function's new behavior.

Missing blank line.
We put a commit message as

$SUMARY
...blank line...
$DESCRIPTION (can contain blank lines)
...blank line...
$TAG block (may not contain blank lines)

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

...

> +			unsigned long res_flags;

Why not calling it irq_flags?

...

> +struct resource;

This...

> +	struct resource r = {};

> +	return ret ?: r.start;

...does _not_ cover these cases.

Hence ioport.h must be included. Did I miss it?

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v4 03/24] i2c: acpi: Modify i2c_acpi_get_irq() to use resource
  2024-01-02 21:07 ` [PATCH v4 03/24] i2c: acpi: Modify i2c_acpi_get_irq() " Mark Hasemeyer
@ 2024-01-06 14:48   ` Andy Shevchenko
  0 siblings, 0 replies; 13+ messages in thread
From: Andy Shevchenko @ 2024-01-06 14:48 UTC (permalink / raw)
  To: Mark Hasemeyer
  Cc: LKML, Sudeep Holla, AngeloGioacchino Del Regno, Rob Herring,
	Krzysztof Kozlowski, Konrad Dybcio, Raul Rangel, Tzung-Bi Shih,
	Mika Westerberg, Wolfram Sang, linux-acpi, linux-i2c

On Tue, Jan 02, 2024 at 02:07:27PM -0700, Mark Hasemeyer wrote:
> The i2c_acpi_irq_context structure provides redundant information that
> can be provided with struct resource.
> 
> Refactor i2c_acpi_get_irq() to use struct resource instead of struct
> i2c_acpi_irq_context.

...

>  	ret = acpi_dev_get_resources(adev, &resource_list,
> -				     i2c_acpi_add_irq_resource, &irq_ctx);
> +				     i2c_acpi_add_irq_resource, r);

Up to you, but you can make it a single line.

	ret = acpi_dev_get_resources(adev, &resource_list, i2c_acpi_add_irq_resource, r);

>  	if (ret < 0)
>  		return ret;

...

> +++ b/drivers/i2c/i2c-core-base.c

> +			struct resource r = {};

Needs ioport.h.

...

> +			irq = i2c_acpi_get_irq(client, &r);
> +			if (r.flags & IORESOURCE_IRQ_WAKECAPABLE)

Ditto.

>  				client->flags |= I2C_CLIENT_WAKE;
>  		}

...

> +++ b/drivers/i2c/i2c-core.h

> +int i2c_acpi_get_irq(struct i2c_client *client, struct resource *r);

> +static inline int i2c_acpi_get_irq(struct i2c_client *client, struct resource *r)

Needs a forward declaration (besides the inclusion block is a total mess
in this file).

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v4 21/24] device property: Modify fwnode irq_get() to use resource
  2024-01-02 21:07 ` [PATCH v4 21/24] device property: Modify fwnode irq_get() " Mark Hasemeyer
@ 2024-01-06 14:52   ` Andy Shevchenko
  2024-01-09 20:47   ` Rob Herring
  1 sibling, 0 replies; 13+ messages in thread
From: Andy Shevchenko @ 2024-01-06 14:52 UTC (permalink / raw)
  To: Mark Hasemeyer
  Cc: LKML, Sudeep Holla, AngeloGioacchino Del Regno, Rob Herring,
	Krzysztof Kozlowski, Konrad Dybcio, Raul Rangel, Tzung-Bi Shih,
	Sakari Ailus, Daniel Scally, Frank Rowand, Greg Kroah-Hartman,
	Heikki Krogerus, Len Brown, Rafael J. Wysocki, Rob Herring,
	devicetree, linux-acpi

On Tue, Jan 02, 2024 at 02:07:45PM -0700, Mark Hasemeyer wrote:
> The underlying ACPI and OF subsystems provide their own APIs which
> provide IRQ information as a struct resource. This allows callers to get
> more information about the IRQ by looking at the resource flags. For
> example, whether or not an IRQ is wake capable.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v4 22/24] device property: Update functions to use EXPORT_SYMBOL_GPL()
  2024-01-02 21:07 ` [PATCH v4 22/24] device property: Update functions to use EXPORT_SYMBOL_GPL() Mark Hasemeyer
@ 2024-01-08 18:32   ` Sakari Ailus
  0 siblings, 0 replies; 13+ messages in thread
From: Sakari Ailus @ 2024-01-08 18:32 UTC (permalink / raw)
  To: Mark Hasemeyer
  Cc: LKML, Sudeep Holla, AngeloGioacchino Del Regno, Rob Herring,
	Andy Shevchenko, Krzysztof Kozlowski, Konrad Dybcio, Raul Rangel,
	Tzung-Bi Shih, Andy Shevchenko, Daniel Scally,
	Greg Kroah-Hartman, Heikki Krogerus, Rafael J. Wysocki,
	linux-acpi

Hi Mark,

On Tue, Jan 02, 2024 at 02:07:46PM -0700, Mark Hasemeyer wrote:
> Some of the exported functions use EXPORT_SYMBOL() instead of
> EXPORT_SYMBOL_GPL() and are inconsistent with the other exported
> functions in the module. The underlying APCI/OF struct fwnode_operations
> implementations are also exported via EXPORT_SYMBOL_GPL().
> 
> Update them to use the EXPORT_SYMBOL_GPL() macro.
> 
> Suggested-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Mark Hasemeyer <markhas@chromium.org>

Thanks!

Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>

-- 
Regards,

Sakari Ailus

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

* Re: [PATCH v4 02/24] gpiolib: acpi: Modify acpi_dev_irq_wake_get_by() to use resource
  2024-01-06 14:44   ` Andy Shevchenko
@ 2024-01-08 19:08     ` Mark Hasemeyer
  0 siblings, 0 replies; 13+ messages in thread
From: Mark Hasemeyer @ 2024-01-08 19:08 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: LKML, Sudeep Holla, AngeloGioacchino Del Regno, Rob Herring,
	Krzysztof Kozlowski, Konrad Dybcio, Raul Rangel, Tzung-Bi Shih,
	Bartosz Golaszewski, Len Brown, Linus Walleij, Mika Westerberg,
	Rafael J. Wysocki, Wolfram Sang, linux-acpi, linux-gpio,
	linux-i2c

> Missing blank line.
> We put a commit message as
>
> $SUMARY
> ...blank line...
> $DESCRIPTION (can contain blank lines)
> ...blank line...
> $TAG block (may not contain blank lines)
>
> > Signed-off-by: Mark Hasemeyer <markhas@chromium.org>

Looks like a nuance of patman I need to iron out.

>
> ...
>
> > +                     unsigned long res_flags;
>
> Why not calling it irq_flags?

irq_flags is already used within the same scope, although it's
declared at the top of the function. I'll move the declaration to the
scope where it's used and rename irq_flags -> irq_type, and irq_res ->
irq_flags.

> > +struct resource;
>
> This...
>
> > +     struct resource r = {};
>
> > +     return ret ?: r.start;
>
> ...does _not_ cover these cases.
>
> Hence ioport.h must be included. Did I miss it?

You're right. It didn't break the build, which means ioport.h must be
included indirectly. I'll add it back.

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

* Re: [PATCH v4 21/24] device property: Modify fwnode irq_get() to use resource
  2024-01-02 21:07 ` [PATCH v4 21/24] device property: Modify fwnode irq_get() " Mark Hasemeyer
  2024-01-06 14:52   ` Andy Shevchenko
@ 2024-01-09 20:47   ` Rob Herring
  1 sibling, 0 replies; 13+ messages in thread
From: Rob Herring @ 2024-01-09 20:47 UTC (permalink / raw)
  To: Mark Hasemeyer
  Cc: Sakari Ailus, AngeloGioacchino Del Regno, Daniel Scally,
	Raul Rangel, Greg Kroah-Hartman, Heikki Krogerus, linux-acpi,
	Andy Shevchenko, Rafael J. Wysocki, Andy Shevchenko, Len Brown,
	devicetree, Konrad Dybcio, Krzysztof Kozlowski, Tzung-Bi Shih,
	LKML, Frank Rowand, Sudeep Holla, Rob Herring


On Tue, 02 Jan 2024 14:07:45 -0700, Mark Hasemeyer wrote:
> The underlying ACPI and OF subsystems provide their own APIs which
> provide IRQ information as a struct resource. This allows callers to get
> more information about the IRQ by looking at the resource flags. For
> example, whether or not an IRQ is wake capable.
> 
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Mark Hasemeyer <markhas@chromium.org>
> ---
> 
> Changes in v4:
> -Add Sakari's Reviewed-by tag from v2
> -Remove ioport.h dependency in fwnode.h
> -Use Andy's @linux.intel.com email
> 
> Changes in v3:
> -Add Suggested-by tag
> -Initialize struct resource to 0 on stack
> -EXPORT_SYMBOL()->EXPORT_SYMBOL_GPL()
> -Remove extra space in commit message
> -Reformat fwnode_irq_get_resource() declaration
> 
> Changes in v2:
> -New patch
> 
>  drivers/acpi/property.c  | 11 +++++------
>  drivers/base/property.c  | 32 +++++++++++++++++++++++++-------
>  drivers/of/property.c    |  8 ++++----
>  include/linux/fwnode.h   |  8 +++++---
>  include/linux/property.h |  2 ++
>  5 files changed, 41 insertions(+), 20 deletions(-)
> 

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


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

* Re: [PATCH v4 00/24] Improve IRQ wake capability reporting and update the cros_ec driver to use it
  2024-01-02 21:07 [PATCH v4 00/24] Improve IRQ wake capability reporting and update the cros_ec driver to use it Mark Hasemeyer
                   ` (3 preceding siblings ...)
  2024-01-02 21:07 ` [PATCH v4 22/24] device property: Update functions to use EXPORT_SYMBOL_GPL() Mark Hasemeyer
@ 2024-01-22  9:14 ` Matthias Brugger
  2024-02-14 17:57 ` (subset) " Bjorn Andersson
  5 siblings, 0 replies; 13+ messages in thread
From: Matthias Brugger @ 2024-01-22  9:14 UTC (permalink / raw)
  To: Mark Hasemeyer, LKML
  Cc: Sudeep Holla, AngeloGioacchino Del Regno, Rob Herring,
	Andy Shevchenko, Krzysztof Kozlowski, Konrad Dybcio, Raul Rangel,
	Tzung-Bi Shih, AKASHI Takahiro, Alexandre TORGUE, Alim Akhtar,
	Andre Przywara, Andrew Morton, Andy Shevchenko, Baoquan He,
	Bartosz Golaszewski, Benson Leung, Bhanu Prakash Maiya,
	Bjorn Andersson, Chen-Yu Tsai, Conor Dooley, Daniel Scally,
	David Gow, Frank Rowand, Greg Kroah-Hartman, Guenter Roeck,
	Heikki Krogerus, Heiko Stuebner, Jonathan Hunter,
	Krzysztof Kozlowski, Len Brown, Linus Walleij, Mark Brown,
	Mika Westerberg, Nick Hawkins, Paul Barker, Prashant Malani,
	Rafael J. Wysocki, Rob Barnes, Rob Herring, Romain Perier,
	Sakari Ailus, Stephen Boyd, Takashi Iwai, Thierry Reding,
	Uwe Kleine-König, Wei Xu, Wolfram Sang, chrome-platform,
	cros-qcom-dts-watchers, devicetree, linux-acpi, linux-arm-kernel,
	linux-arm-msm, linux-gpio, linux-i2c, linux-mediatek,
	linux-rockchip, linux-samsung-soc, linux-tegra



On 02/01/2024 22:07, Mark Hasemeyer wrote:
> Currently the cros_ec driver assumes that its associated interrupt is
> wake capable. This is an incorrect assumption as some Chromebooks use a
> separate wake pin, while others overload the interrupt for wake and IO.
> This patch train updates the driver to query the underlying ACPI/DT data
> to determine whether or not the IRQ should be enabled for wake.
> 
> Both the device tree and ACPI systems have methods for reporting IRQ
> wake capability. In device tree based systems, a node can advertise
> itself as a 'wakeup-source'. In ACPI based systems, GpioInt and
> Interrupt resource descriptors can use the 'SharedAndWake' or
> 'ExclusiveAndWake' share types.
> 
> Some logic is added to the platform, ACPI, and DT subsystems to more
> easily pipe wakeirq information up to the driver.
> 

Patch 9, 10, 11 and 12 applied to the mediatek tree.

Thanks!


> Changes in v4:
> -Rebase on linux-next
> -See each patch for patch specific changes
> 
> Changes in v3:
> -Rebase on linux-next
> -See each patch for patch specific changes
> 
> Changes in v2:
> -Rebase on linux-next
> -Add cover letter
> -See each patch for patch specific changes
> 
> Mark Hasemeyer (24):
>    resource: Add DEFINE_RES_*_NAMED_FLAGS macro
>    gpiolib: acpi: Modify acpi_dev_irq_wake_get_by() to use resource
>    i2c: acpi: Modify i2c_acpi_get_irq() to use resource
>    dt-bindings: power: Clarify wording for wakeup-source property
>    ARM: dts: tegra: Enable cros-ec-spi as wake source
>    ARM: dts: rockchip: rk3288: Enable cros-ec-spi as wake source
>    ARM: dts: samsung: exynos5420: Enable cros-ec-spi as wake source
>    ARM: dts: samsung: exynos5800: Enable cros-ec-spi as wake source
>    arm64: dts: mediatek: mt8173: Enable cros-ec-spi as wake source
>    arm64: dts: mediatek: mt8183: Enable cros-ec-spi as wake source
>    arm64: dts: mediatek: mt8192: Enable cros-ec-spi as wake source
>    arm64: dts: mediatek: mt8195: Enable cros-ec-spi as wake source
>    arm64: dts: tegra: Enable cros-ec-spi as wake source
>    arm64: dts: qcom: sc7180: Enable cros-ec-spi as wake source
>    arm64: dts: qcom: sc7280: Enable cros-ec-spi as wake source
>    arm64: dts: qcom: sdm845: Enable cros-ec-spi as wake source
>    arm64: dts: rockchip: rk3399: Enable cros-ec-spi as wake source
>    of: irq: add wake capable bit to of_irq_resource()
>    of: irq: Add default implementation for of_irq_to_resource()
>    of: irq: Remove extern from function declarations
>    device property: Modify fwnode irq_get() to use resource
>    device property: Update functions to use EXPORT_SYMBOL_GPL()
>    platform: Modify platform_get_irq_optional() to use resource
>    platform/chrome: cros_ec: Use PM subsystem to manage wakeirq
> 
>   .../bindings/power/wakeup-source.txt          | 18 ++--
>   arch/arm/boot/dts/nvidia/tegra124-nyan.dtsi   |  1 +
>   arch/arm/boot/dts/nvidia/tegra124-venice2.dts |  1 +
>   .../rockchip/rk3288-veyron-chromebook.dtsi    |  1 +
>   .../boot/dts/samsung/exynos5420-peach-pit.dts |  1 +
>   .../boot/dts/samsung/exynos5800-peach-pi.dts  |  1 +
>   arch/arm64/boot/dts/mediatek/mt8173-elm.dtsi  |  1 +
>   .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi |  1 +
>   .../boot/dts/mediatek/mt8192-asurada.dtsi     |  1 +
>   .../boot/dts/mediatek/mt8195-cherry.dtsi      |  1 +
>   .../arm64/boot/dts/nvidia/tegra132-norrin.dts |  1 +
>   arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi  |  1 +
>   .../arm64/boot/dts/qcom/sc7280-herobrine.dtsi |  1 +
>   .../arm64/boot/dts/qcom/sc7280-idp-ec-h1.dtsi |  1 +
>   arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi    |  1 +
>   arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi  |  1 +
>   drivers/acpi/property.c                       | 11 ++-
>   drivers/base/platform.c                       | 90 ++++++++++++-------
>   drivers/base/property.c                       | 40 ++++++---
>   drivers/gpio/gpiolib-acpi.c                   | 28 ++++--
>   drivers/i2c/i2c-core-acpi.c                   | 43 ++++-----
>   drivers/i2c/i2c-core-base.c                   |  6 +-
>   drivers/i2c/i2c-core.h                        |  4 +-
>   drivers/of/irq.c                              | 39 +++++++-
>   drivers/of/property.c                         |  8 +-
>   drivers/platform/chrome/cros_ec.c             | 48 ++++++++--
>   drivers/platform/chrome/cros_ec_lpc.c         | 40 +++++++--
>   drivers/platform/chrome/cros_ec_spi.c         | 15 ++--
>   drivers/platform/chrome/cros_ec_uart.c        | 14 ++-
>   include/linux/acpi.h                          | 25 +++---
>   include/linux/fwnode.h                        |  8 +-
>   include/linux/ioport.h                        | 20 +++--
>   include/linux/of_irq.h                        | 41 +++++----
>   include/linux/platform_data/cros_ec_proto.h   |  4 +-
>   include/linux/platform_device.h               |  3 +
>   include/linux/property.h                      |  2 +
>   36 files changed, 350 insertions(+), 172 deletions(-)
> 

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

* Re: (subset) [PATCH v4 00/24] Improve IRQ wake capability reporting and update the cros_ec driver to use it
  2024-01-02 21:07 [PATCH v4 00/24] Improve IRQ wake capability reporting and update the cros_ec driver to use it Mark Hasemeyer
                   ` (4 preceding siblings ...)
  2024-01-22  9:14 ` [PATCH v4 00/24] Improve IRQ wake capability reporting and update the cros_ec driver to use it Matthias Brugger
@ 2024-02-14 17:57 ` Bjorn Andersson
  5 siblings, 0 replies; 13+ messages in thread
From: Bjorn Andersson @ 2024-02-14 17:57 UTC (permalink / raw)
  To: LKML, Mark Hasemeyer
  Cc: Sudeep Holla, AngeloGioacchino Del Regno, Rob Herring,
	Andy Shevchenko, Krzysztof Kozlowski, Konrad Dybcio, Raul Rangel,
	Tzung-Bi Shih, AKASHI Takahiro, Alexandre TORGUE, Alim Akhtar,
	Andre Przywara, Andrew Morton, Andy Shevchenko, Baoquan He,
	Bartosz Golaszewski, Benson Leung, Bhanu Prakash Maiya,
	Chen-Yu Tsai, Conor Dooley, Daniel Scally, David Gow,
	Frank Rowand, Greg Kroah-Hartman, Guenter Roeck, Heikki Krogerus,
	Heiko Stuebner, Jonathan Hunter, Krzysztof Kozlowski, Len Brown,
	Linus Walleij, Mark Brown, Matthias Brugger, Mika Westerberg,
	Nick Hawkins, Paul Barker, Prashant Malani, Rafael J. Wysocki,
	Rob Barnes, Rob Herring, Romain Perier, Sakari Ailus,
	Stephen Boyd, Takashi Iwai, Thierry Reding,
	Uwe Kleine-König, Wei Xu, Wolfram Sang, chrome-platform,
	cros-qcom-dts-watchers, devicetree, linux-acpi, linux-arm-kernel,
	linux-arm-msm, linux-gpio, linux-i2c, linux-mediatek,
	linux-rockchip, linux-samsung-soc, linux-tegra


On Tue, 02 Jan 2024 14:07:24 -0700, Mark Hasemeyer wrote:
> Currently the cros_ec driver assumes that its associated interrupt is
> wake capable. This is an incorrect assumption as some Chromebooks use a
> separate wake pin, while others overload the interrupt for wake and IO.
> This patch train updates the driver to query the underlying ACPI/DT data
> to determine whether or not the IRQ should be enabled for wake.
> 
> Both the device tree and ACPI systems have methods for reporting IRQ
> wake capability. In device tree based systems, a node can advertise
> itself as a 'wakeup-source'. In ACPI based systems, GpioInt and
> Interrupt resource descriptors can use the 'SharedAndWake' or
> 'ExclusiveAndWake' share types.
> 
> [...]

Applied, thanks!

[14/24] arm64: dts: qcom: sc7180: Enable cros-ec-spi as wake source
        commit: f172a341ec1f66bac2866720931594e81f02ad4d
[15/24] arm64: dts: qcom: sc7280: Enable cros-ec-spi as wake source
        commit: a4b28b9ecc99673da875e214b1a06f1e0f0a24fa
[16/24] arm64: dts: qcom: sdm845: Enable cros-ec-spi as wake source
        commit: a7baa25bfbfdcd4e76414f29ab43317ded8d3e6e

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>

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

end of thread, other threads:[~2024-02-14 17:58 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-02 21:07 [PATCH v4 00/24] Improve IRQ wake capability reporting and update the cros_ec driver to use it Mark Hasemeyer
2024-01-02 21:07 ` [PATCH v4 02/24] gpiolib: acpi: Modify acpi_dev_irq_wake_get_by() to use resource Mark Hasemeyer
2024-01-06 14:44   ` Andy Shevchenko
2024-01-08 19:08     ` Mark Hasemeyer
2024-01-02 21:07 ` [PATCH v4 03/24] i2c: acpi: Modify i2c_acpi_get_irq() " Mark Hasemeyer
2024-01-06 14:48   ` Andy Shevchenko
2024-01-02 21:07 ` [PATCH v4 21/24] device property: Modify fwnode irq_get() " Mark Hasemeyer
2024-01-06 14:52   ` Andy Shevchenko
2024-01-09 20:47   ` Rob Herring
2024-01-02 21:07 ` [PATCH v4 22/24] device property: Update functions to use EXPORT_SYMBOL_GPL() Mark Hasemeyer
2024-01-08 18:32   ` Sakari Ailus
2024-01-22  9:14 ` [PATCH v4 00/24] Improve IRQ wake capability reporting and update the cros_ec driver to use it Matthias Brugger
2024-02-14 17:57 ` (subset) " Bjorn Andersson

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