* [PATCH v2 01/22] gpiolib: acpi: Modify acpi_dev_irq_wake_get_by() to use resource
2023-12-20 23:54 [PATCH v2 00/22] Improve IRQ wake capability reporting and update the cros_ec driver to use it Mark Hasemeyer
@ 2023-12-20 23:54 ` Mark Hasemeyer
2023-12-21 15:17 ` Andy Shevchenko
2023-12-23 2:05 ` kernel test robot
2023-12-20 23:54 ` [PATCH v2 02/22] i2c: acpi: Modify i2c_acpi_get_irq() " Mark Hasemeyer
` (2 subsequent siblings)
3 siblings, 2 replies; 20+ messages in thread
From: Mark Hasemeyer @ 2023-12-20 23:54 UTC (permalink / raw)
To: LKML
Cc: AngeloGioacchino Del Regno, Krzysztof Kozlowski, Tzung-Bi Shih,
Raul Rangel, Konrad Dybcio, Andy Shevchenko, Rob Herring,
Sudeep Holla, 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 v2:
-Remove explicit cast to struct resource
-irq -> IRQ
drivers/gpio/gpiolib-acpi.c | 25 ++++++++++++++++---------
drivers/i2c/i2c-core-acpi.c | 10 ++++++++--
include/linux/acpi.h | 23 ++++++++++-------------
3 files changed, 34 insertions(+), 24 deletions(-)
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 88066826d8e5b..ef98b50f42f86 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -111,6 +111,7 @@ struct acpi_gpio_info {
int polarity;
int triggering;
bool wake_capable;
+ bool shareable;
unsigned int debounce;
unsigned int quirks;
};
@@ -760,6 +761,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 +1006,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 +1025,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;
@@ -1084,16 +1088,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;
+ *r = DEFINE_RES_IRQ(irq);
+ r->flags = acpi_dev_irq_flags(info.triggering, info.polarity,
+ info.shareable, info.wake_capable);
+ 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 118a18b7ff844..83aa2fa8e81fc 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1221,8 +1221,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)
@@ -1234,28 +1234,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] 20+ messages in thread* Re: [PATCH v2 01/22] gpiolib: acpi: Modify acpi_dev_irq_wake_get_by() to use resource
2023-12-20 23:54 ` [PATCH v2 01/22] gpiolib: acpi: Modify acpi_dev_irq_wake_get_by() to use resource Mark Hasemeyer
@ 2023-12-21 15:17 ` Andy Shevchenko
2023-12-23 2:05 ` kernel test robot
1 sibling, 0 replies; 20+ messages in thread
From: Andy Shevchenko @ 2023-12-21 15:17 UTC (permalink / raw)
To: Mark Hasemeyer
Cc: LKML, AngeloGioacchino Del Regno, Krzysztof Kozlowski,
Tzung-Bi Shih, Raul Rangel, Konrad Dybcio, Rob Herring,
Sudeep Holla, Bartosz Golaszewski, Len Brown, Linus Walleij,
Mika Westerberg, Rafael J. Wysocki, Wolfram Sang, linux-acpi,
linux-gpio, linux-i2c
On Wed, Dec 20, 2023 at 04:54:15PM -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.
...
> + *r = DEFINE_RES_IRQ(irq);
> + r->flags = acpi_dev_irq_flags(info.triggering, info.polarity,
> + info.shareable, info.wake_capable);
Looking at this I am wondering if we can actually to have
unsigned long irq_flags;
...
irq_flags = acpi_dev_irq_flags(info.triggering, info.polarity,
info.shareable, info.wake_capable);
*r = DEFINE_RES_NAMED(irq, 1, NULL, irq_flags);
as we don't need to duplicate IORESOURCE_IRQ.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 01/22] gpiolib: acpi: Modify acpi_dev_irq_wake_get_by() to use resource
2023-12-20 23:54 ` [PATCH v2 01/22] gpiolib: acpi: Modify acpi_dev_irq_wake_get_by() to use resource Mark Hasemeyer
2023-12-21 15:17 ` Andy Shevchenko
@ 2023-12-23 2:05 ` kernel test robot
2023-12-23 3:09 ` Mark Hasemeyer
1 sibling, 1 reply; 20+ messages in thread
From: kernel test robot @ 2023-12-23 2:05 UTC (permalink / raw)
To: Mark Hasemeyer, LKML
Cc: oe-kbuild-all, AngeloGioacchino Del Regno, Krzysztof Kozlowski,
Tzung-Bi Shih, Raul Rangel, Konrad Dybcio, Andy Shevchenko,
Rob Herring, Sudeep Holla, Mark Hasemeyer, Bartosz Golaszewski,
Len Brown, Linus Walleij, Mika Westerberg, Rafael J. Wysocki,
Wolfram Sang, linux-acpi, linux-gpio, linux-i2c
Hi Mark,
kernel test robot noticed the following build warnings:
[auto build test WARNING on robh/for-next]
[also build test WARNING on chrome-platform/for-next chrome-platform/for-firmware-next wsa/i2c/for-next driver-core/driver-core-testing driver-core/driver-core-next driver-core/driver-core-linus linus/master v6.7-rc6 next-20231222]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Mark-Hasemeyer/gpiolib-acpi-Modify-acpi_dev_irq_wake_get_by-to-use-resource/20231222-172104
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link: https://lore.kernel.org/r/20231220165423.v2.1.Ifd0903f1c351e84376d71dbdadbd43931197f5ea%40changeid
patch subject: [PATCH v2 01/22] gpiolib: acpi: Modify acpi_dev_irq_wake_get_by() to use resource
config: x86_64-randconfig-161-20231222 (https://download.01.org/0day-ci/archive/20231223/202312230907.szXqJyXq-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231223/202312230907.szXqJyXq-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312230907.szXqJyXq-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/gpio/gpiolib-acpi.c:117: warning: Function parameter or member 'shareable' not described in 'acpi_gpio_info'
vim +117 drivers/gpio/gpiolib-acpi.c
aa92b6f689acf1 Mika Westerberg 2014-03-10 93
b7452d670fdef8 Dmitry Torokhov 2022-11-15 94 /**
b7452d670fdef8 Dmitry Torokhov 2022-11-15 95 * struct acpi_gpio_info - ACPI GPIO specific information
b7452d670fdef8 Dmitry Torokhov 2022-11-15 96 * @adev: reference to ACPI device which consumes GPIO resource
b7452d670fdef8 Dmitry Torokhov 2022-11-15 97 * @flags: GPIO initialization flags
b7452d670fdef8 Dmitry Torokhov 2022-11-15 98 * @gpioint: if %true this GPIO is of type GpioInt otherwise type is GpioIo
b7452d670fdef8 Dmitry Torokhov 2022-11-15 99 * @pin_config: pin bias as provided by ACPI
b7452d670fdef8 Dmitry Torokhov 2022-11-15 100 * @polarity: interrupt polarity as provided by ACPI
b7452d670fdef8 Dmitry Torokhov 2022-11-15 101 * @triggering: triggering type as provided by ACPI
b7452d670fdef8 Dmitry Torokhov 2022-11-15 102 * @wake_capable: wake capability as provided by ACPI
b7452d670fdef8 Dmitry Torokhov 2022-11-15 103 * @debounce: debounce timeout as provided by ACPI
b7452d670fdef8 Dmitry Torokhov 2022-11-15 104 * @quirks: Linux specific quirks as provided by struct acpi_gpio_mapping
b7452d670fdef8 Dmitry Torokhov 2022-11-15 105 */
b7452d670fdef8 Dmitry Torokhov 2022-11-15 106 struct acpi_gpio_info {
b7452d670fdef8 Dmitry Torokhov 2022-11-15 107 struct acpi_device *adev;
b7452d670fdef8 Dmitry Torokhov 2022-11-15 108 enum gpiod_flags flags;
b7452d670fdef8 Dmitry Torokhov 2022-11-15 109 bool gpioint;
b7452d670fdef8 Dmitry Torokhov 2022-11-15 110 int pin_config;
b7452d670fdef8 Dmitry Torokhov 2022-11-15 111 int polarity;
b7452d670fdef8 Dmitry Torokhov 2022-11-15 112 int triggering;
b7452d670fdef8 Dmitry Torokhov 2022-11-15 113 bool wake_capable;
189f4620fa2d51 Mark Hasemeyer 2023-12-20 114 bool shareable;
b7452d670fdef8 Dmitry Torokhov 2022-11-15 115 unsigned int debounce;
b7452d670fdef8 Dmitry Torokhov 2022-11-15 116 unsigned int quirks;
b7452d670fdef8 Dmitry Torokhov 2022-11-15 @117 };
b7452d670fdef8 Dmitry Torokhov 2022-11-15 118
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 01/22] gpiolib: acpi: Modify acpi_dev_irq_wake_get_by() to use resource
2023-12-23 2:05 ` kernel test robot
@ 2023-12-23 3:09 ` Mark Hasemeyer
0 siblings, 0 replies; 20+ messages in thread
From: Mark Hasemeyer @ 2023-12-23 3:09 UTC (permalink / raw)
To: kernel test robot
Cc: LKML, oe-kbuild-all, AngeloGioacchino Del Regno,
Krzysztof Kozlowski, Tzung-Bi Shih, Raul Rangel, Konrad Dybcio,
Andy Shevchenko, Rob Herring, Sudeep Holla, Bartosz Golaszewski,
Len Brown, Linus Walleij, Mika Westerberg, Rafael J. Wysocki,
Wolfram Sang, linux-acpi, linux-gpio, linux-i2c
On Fri, Dec 22, 2023 at 7:09 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi Mark,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on robh/for-next]
> [also build test WARNING on chrome-platform/for-next chrome-platform/for-firmware-next wsa/i2c/for-next driver-core/driver-core-testing driver-core/driver-core-next driver-core/driver-core-linus linus/master v6.7-rc6 next-20231222]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Mark-Hasemeyer/gpiolib-acpi-Modify-acpi_dev_irq_wake_get_by-to-use-resource/20231222-172104
> base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
> patch link: https://lore.kernel.org/r/20231220165423.v2.1.Ifd0903f1c351e84376d71dbdadbd43931197f5ea%40changeid
> patch subject: [PATCH v2 01/22] gpiolib: acpi: Modify acpi_dev_irq_wake_get_by() to use resource
> config: x86_64-randconfig-161-20231222 (https://download.01.org/0day-ci/archive/20231223/202312230907.szXqJyXq-lkp@intel.com/config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231223/202312230907.szXqJyXq-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202312230907.szXqJyXq-lkp@intel.com/
>
> All warnings (new ones prefixed by >>):
>
> >> drivers/gpio/gpiolib-acpi.c:117: warning: Function parameter or member 'shareable' not described in 'acpi_gpio_info'
>
>
> vim +117 drivers/gpio/gpiolib-acpi.c
>
> aa92b6f689acf1 Mika Westerberg 2014-03-10 93
> b7452d670fdef8 Dmitry Torokhov 2022-11-15 94 /**
> b7452d670fdef8 Dmitry Torokhov 2022-11-15 95 * struct acpi_gpio_info - ACPI GPIO specific information
> b7452d670fdef8 Dmitry Torokhov 2022-11-15 96 * @adev: reference to ACPI device which consumes GPIO resource
> b7452d670fdef8 Dmitry Torokhov 2022-11-15 97 * @flags: GPIO initialization flags
> b7452d670fdef8 Dmitry Torokhov 2022-11-15 98 * @gpioint: if %true this GPIO is of type GpioInt otherwise type is GpioIo
> b7452d670fdef8 Dmitry Torokhov 2022-11-15 99 * @pin_config: pin bias as provided by ACPI
> b7452d670fdef8 Dmitry Torokhov 2022-11-15 100 * @polarity: interrupt polarity as provided by ACPI
> b7452d670fdef8 Dmitry Torokhov 2022-11-15 101 * @triggering: triggering type as provided by ACPI
> b7452d670fdef8 Dmitry Torokhov 2022-11-15 102 * @wake_capable: wake capability as provided by ACPI
> b7452d670fdef8 Dmitry Torokhov 2022-11-15 103 * @debounce: debounce timeout as provided by ACPI
> b7452d670fdef8 Dmitry Torokhov 2022-11-15 104 * @quirks: Linux specific quirks as provided by struct acpi_gpio_mapping
> b7452d670fdef8 Dmitry Torokhov 2022-11-15 105 */
> b7452d670fdef8 Dmitry Torokhov 2022-11-15 106 struct acpi_gpio_info {
> b7452d670fdef8 Dmitry Torokhov 2022-11-15 107 struct acpi_device *adev;
> b7452d670fdef8 Dmitry Torokhov 2022-11-15 108 enum gpiod_flags flags;
> b7452d670fdef8 Dmitry Torokhov 2022-11-15 109 bool gpioint;
> b7452d670fdef8 Dmitry Torokhov 2022-11-15 110 int pin_config;
> b7452d670fdef8 Dmitry Torokhov 2022-11-15 111 int polarity;
> b7452d670fdef8 Dmitry Torokhov 2022-11-15 112 int triggering;
> b7452d670fdef8 Dmitry Torokhov 2022-11-15 113 bool wake_capable;
> 189f4620fa2d51 Mark Hasemeyer 2023-12-20 114 bool shareable;
> b7452d670fdef8 Dmitry Torokhov 2022-11-15 115 unsigned int debounce;
> b7452d670fdef8 Dmitry Torokhov 2022-11-15 116 unsigned int quirks;
> b7452d670fdef8 Dmitry Torokhov 2022-11-15 @117 };
> b7452d670fdef8 Dmitry Torokhov 2022-11-15 118
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
Ack. Missing documentation for acpi_gpio_info.shareable member. Will
add in next version.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 02/22] i2c: acpi: Modify i2c_acpi_get_irq() to use resource
2023-12-20 23:54 [PATCH v2 00/22] Improve IRQ wake capability reporting and update the cros_ec driver to use it Mark Hasemeyer
2023-12-20 23:54 ` [PATCH v2 01/22] gpiolib: acpi: Modify acpi_dev_irq_wake_get_by() to use resource Mark Hasemeyer
@ 2023-12-20 23:54 ` Mark Hasemeyer
2023-12-21 13:51 ` Andy Shevchenko
2023-12-20 23:54 ` [PATCH v2 20/22] device property: Modify fwnode irq_get() " Mark Hasemeyer
2023-12-21 13:42 ` [PATCH v2 00/22] Improve IRQ wake capability reporting and update the cros_ec driver to use it Andy Shevchenko
3 siblings, 1 reply; 20+ messages in thread
From: Mark Hasemeyer @ 2023-12-20 23:54 UTC (permalink / raw)
To: LKML
Cc: AngeloGioacchino Del Regno, Krzysztof Kozlowski, Tzung-Bi Shih,
Raul Rangel, Konrad Dybcio, Andy Shevchenko, Rob Herring,
Sudeep Holla, Mark Hasemeyer, 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.
Signed-off-by: Mark Hasemeyer <markhas@chromium.org>
---
Changes in v2:
-New patch
drivers/i2c/i2c-core-acpi.c | 44 ++++++++++++++-----------------------
drivers/i2c/i2c-core-base.c | 6 ++---
drivers/i2c/i2c-core.h | 4 ++--
3 files changed, 22 insertions(+), 32 deletions(-)
diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c
index 8126a87baf3d4..01cf140da21af 100644
--- a/drivers/i2c/i2c-core-acpi.c
+++ b/drivers/i2c/i2c-core-acpi.c
@@ -175,64 +175,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->start > 0)
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 (IS_ERR_OR_NULL(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 (irq_ctx.irq < 0)
- return irq_ctx.irq;
+ if (r->start > 0)
+ return r->start;
- if (wake_capable)
- *wake_capable = irq_ctx.wake_capable;
+ ret = acpi_dev_get_gpio_irq_resource(adev, NULL, 0, r);
+ if (!ret)
+ return r->start;
- return irq_ctx.irq;
+ return ret;
}
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..8b8c7581a60c2 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 = {0};
- irq = i2c_acpi_get_irq(client, &wake_capable);
- if (irq > 0 && wake_capable)
+ irq = i2c_acpi_get_irq(client, &r);
+ if (irq > 0 && 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] 20+ messages in thread* Re: [PATCH v2 02/22] i2c: acpi: Modify i2c_acpi_get_irq() to use resource
2023-12-20 23:54 ` [PATCH v2 02/22] i2c: acpi: Modify i2c_acpi_get_irq() " Mark Hasemeyer
@ 2023-12-21 13:51 ` Andy Shevchenko
0 siblings, 0 replies; 20+ messages in thread
From: Andy Shevchenko @ 2023-12-21 13:51 UTC (permalink / raw)
To: Mark Hasemeyer
Cc: LKML, AngeloGioacchino Del Regno, Krzysztof Kozlowski,
Tzung-Bi Shih, Raul Rangel, Konrad Dybcio, Rob Herring,
Sudeep Holla, Mika Westerberg, Wolfram Sang, linux-acpi,
linux-i2c
On Wed, Dec 20, 2023 at 04:54:16PM -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.
Suggested-by?
...
> 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->start > 0)
> return 1;
Checking flags is more robust.
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 */
> }
...
> + if (IS_ERR_OR_NULL(r))
> + return -EINVAL;
Hmm... Do we expect this to be an error pointer in some cases?
...
> + ret = acpi_dev_get_gpio_irq_resource(adev, NULL, 0, r);
> + if (!ret)
> + return r->start;
>
> - return irq_ctx.irq;
> + return ret;
What's wrong with the standard pattern?
if (ret)
return ret;
...
return ...;
...
> + struct resource r = {0};
'0' is redundant.
...
> + irq = i2c_acpi_get_irq(client, &r);
> + if (irq > 0 && r.flags & IORESOURCE_IRQ_WAKECAPABLE)
Why checking just flags is not enough?
> client->flags |= I2C_CLIENT_WAKE;
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 20/22] device property: Modify fwnode irq_get() to use resource
2023-12-20 23:54 [PATCH v2 00/22] Improve IRQ wake capability reporting and update the cros_ec driver to use it Mark Hasemeyer
2023-12-20 23:54 ` [PATCH v2 01/22] gpiolib: acpi: Modify acpi_dev_irq_wake_get_by() to use resource Mark Hasemeyer
2023-12-20 23:54 ` [PATCH v2 02/22] i2c: acpi: Modify i2c_acpi_get_irq() " Mark Hasemeyer
@ 2023-12-20 23:54 ` Mark Hasemeyer
2023-12-21 10:37 ` Sakari Ailus
2023-12-21 13:56 ` Andy Shevchenko
2023-12-21 13:42 ` [PATCH v2 00/22] Improve IRQ wake capability reporting and update the cros_ec driver to use it Andy Shevchenko
3 siblings, 2 replies; 20+ messages in thread
From: Mark Hasemeyer @ 2023-12-20 23:54 UTC (permalink / raw)
To: LKML
Cc: AngeloGioacchino Del Regno, Krzysztof Kozlowski, Tzung-Bi Shih,
Raul Rangel, Konrad Dybcio, Andy Shevchenko, Rob Herring,
Sudeep Holla, Mark Hasemeyer, Andy Shevchenko, Daniel Scally,
Frank Rowand, Greg Kroah-Hartman, Heikki Krogerus, Len Brown,
Rafael J. Wysocki, Rob Herring, Sakari Ailus, 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.
Signed-off-by: Mark Hasemeyer <markhas@chromium.org>
---
Changes in v2:
-New patch
drivers/acpi/property.c | 11 +++++------
drivers/base/property.c | 24 +++++++++++++++++++++---
drivers/of/property.c | 8 ++++----
include/linux/fwnode.h | 8 +++++---
include/linux/property.h | 2 ++
5 files changed, 37 insertions(+), 16 deletions(-)
diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index a6ead5204046b..259869987ffff 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..4f5d5ab5ab8cf 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1047,23 +1047,41 @@ void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int index)
EXPORT_SYMBOL(fwnode_iomap);
/**
- * fwnode_irq_get - Get IRQ directly from a fwnode
+ * 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(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 ret;
- ret = fwnode_call_int_op(fwnode, irq_get, index);
+ 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(fwnode_irq_get_resource);
+
+/**
+ * fwnode_irq_get - Get IRQ directly from a fwnode
+ * @fwnode: Pointer to the firmware node
+ * @index: Zero-based index of the IRQ
+ *
+ * Return: Linux IRQ number on success. Negative errno on failure.
+ */
+int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index)
+{
+ struct resource r;
+
+ 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..716ed863acde0 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -9,10 +9,11 @@
#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/ioport.h>
+#include <linux/list.h>
+#include <linux/types.h>
struct fwnode_operations;
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] 20+ messages in thread* Re: [PATCH v2 20/22] device property: Modify fwnode irq_get() to use resource
2023-12-20 23:54 ` [PATCH v2 20/22] device property: Modify fwnode irq_get() " Mark Hasemeyer
@ 2023-12-21 10:37 ` Sakari Ailus
2023-12-21 23:52 ` Mark Hasemeyer
2023-12-21 13:56 ` Andy Shevchenko
1 sibling, 1 reply; 20+ messages in thread
From: Sakari Ailus @ 2023-12-21 10:37 UTC (permalink / raw)
To: Mark Hasemeyer
Cc: LKML, AngeloGioacchino Del Regno, Krzysztof Kozlowski,
Tzung-Bi Shih, Raul Rangel, Konrad Dybcio, Andy Shevchenko,
Rob Herring, Sudeep Holla, Andy Shevchenko, Daniel Scally,
Frank Rowand, Greg Kroah-Hartman, Heikki Krogerus, Len Brown,
Rafael J. Wysocki, Rob Herring, devicetree, linux-acpi
Hi Mark,
Thank you for the patch.
On Wed, Dec 20, 2023 at 04:54:34PM -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.
>
> Signed-off-by: Mark Hasemeyer <markhas@chromium.org>
> ---
>
> Changes in v2:
> -New patch
>
> drivers/acpi/property.c | 11 +++++------
> drivers/base/property.c | 24 +++++++++++++++++++++---
> drivers/of/property.c | 8 ++++----
> include/linux/fwnode.h | 8 +++++---
> include/linux/property.h | 2 ++
> 5 files changed, 37 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> index a6ead5204046b..259869987ffff 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..4f5d5ab5ab8cf 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -1047,23 +1047,41 @@ void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int index)
> EXPORT_SYMBOL(fwnode_iomap);
>
> /**
> - * fwnode_irq_get - Get IRQ directly from a fwnode
> + * 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(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 ret;
>
> - ret = fwnode_call_int_op(fwnode, irq_get, index);
> + 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(fwnode_irq_get_resource);
Both acpi_irq_get() and of_irq_to_resourece() use EXPORT_SYMBOL_GPL()
instead. I don't see why fwnode_irq_get_resource() shouldn't do the same.
With this changed,
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
In fact this should have always been the case for fwnode_irq_get(). I
wouldn't mind changing that, too, in a separate patch.
> +
> +/**
> + * fwnode_irq_get - Get IRQ directly from a fwnode
> + * @fwnode: Pointer to the firmware node
> + * @index: Zero-based index of the IRQ
> + *
> + * Return: Linux IRQ number on success. Negative errno on failure.
> + */
> +int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index)
> +{
> + struct resource r;
> +
> + 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..716ed863acde0 100644
> --- a/include/linux/fwnode.h
> +++ b/include/linux/fwnode.h
> @@ -9,10 +9,11 @@
> #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/ioport.h>
> +#include <linux/list.h>
> +#include <linux/types.h>
>
> struct fwnode_operations;
> 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);
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 20/22] device property: Modify fwnode irq_get() to use resource
2023-12-21 10:37 ` Sakari Ailus
@ 2023-12-21 23:52 ` Mark Hasemeyer
2023-12-22 12:48 ` Andy Shevchenko
0 siblings, 1 reply; 20+ messages in thread
From: Mark Hasemeyer @ 2023-12-21 23:52 UTC (permalink / raw)
To: Sakari Ailus
Cc: LKML, AngeloGioacchino Del Regno, Krzysztof Kozlowski,
Tzung-Bi Shih, Raul Rangel, Konrad Dybcio, Andy Shevchenko,
Rob Herring, Sudeep Holla, Andy Shevchenko, Daniel Scally,
Frank Rowand, Greg Kroah-Hartman, Heikki Krogerus, Len Brown,
Rafael J. Wysocki, Rob Herring, devicetree, linux-acpi
> Both acpi_irq_get() and of_irq_to_resourece() use EXPORT_SYMBOL_GPL()
> instead. I don't see why fwnode_irq_get_resource() shouldn't do the same.
>
> With this changed,
>
> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>
> In fact this should have always been the case for fwnode_irq_get(). I
> wouldn't mind changing that, too, in a separate patch.
Sure. It looks like fwnode_iomap(), fwnode_irq_get(),
fwnode_irq_get_byname(), and fwnode_graph_parse_endpoint() could all
be updated. I'll add another patch with these changes unless there's a
reason some of those functions shouldn't be GPL'd?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 20/22] device property: Modify fwnode irq_get() to use resource
2023-12-21 23:52 ` Mark Hasemeyer
@ 2023-12-22 12:48 ` Andy Shevchenko
0 siblings, 0 replies; 20+ messages in thread
From: Andy Shevchenko @ 2023-12-22 12:48 UTC (permalink / raw)
To: Mark Hasemeyer
Cc: Sakari Ailus, LKML, AngeloGioacchino Del Regno,
Krzysztof Kozlowski, Tzung-Bi Shih, Raul Rangel, Konrad Dybcio,
Rob Herring, Sudeep Holla, Daniel Scally, Frank Rowand,
Greg Kroah-Hartman, Heikki Krogerus, Len Brown, Rafael J. Wysocki,
Rob Herring, devicetree, linux-acpi
On Thu, Dec 21, 2023 at 04:52:15PM -0700, Mark Hasemeyer wrote:
> > Both acpi_irq_get() and of_irq_to_resourece() use EXPORT_SYMBOL_GPL()
> > instead. I don't see why fwnode_irq_get_resource() shouldn't do the same.
> >
> > With this changed,
> >
> > Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> >
> > In fact this should have always been the case for fwnode_irq_get(). I
> > wouldn't mind changing that, too, in a separate patch.
>
> Sure. It looks like fwnode_iomap(), fwnode_irq_get(),
> fwnode_irq_get_byname(), and fwnode_graph_parse_endpoint() could all
> be updated. I'll add another patch with these changes unless there's a
> reason some of those functions shouldn't be GPL'd?
Interesting... Once should look at the history of those changes.
I don't think anything prevents us from moving to GPLed versions.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 20/22] device property: Modify fwnode irq_get() to use resource
2023-12-20 23:54 ` [PATCH v2 20/22] device property: Modify fwnode irq_get() " Mark Hasemeyer
2023-12-21 10:37 ` Sakari Ailus
@ 2023-12-21 13:56 ` Andy Shevchenko
2023-12-21 23:46 ` Mark Hasemeyer
1 sibling, 1 reply; 20+ messages in thread
From: Andy Shevchenko @ 2023-12-21 13:56 UTC (permalink / raw)
To: Mark Hasemeyer
Cc: LKML, AngeloGioacchino Del Regno, Krzysztof Kozlowski,
Tzung-Bi Shih, Raul Rangel, Konrad Dybcio, Rob Herring,
Sudeep Holla, Daniel Scally, Frank Rowand, Greg Kroah-Hartman,
Heikki Krogerus, Len Brown, Rafael J. Wysocki, Rob Herring,
Sakari Ailus, devicetree, linux-acpi
On Wed, Dec 20, 2023 at 04:54:34PM -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
Double space when other lines have a single space.
> example, whether or not an IRQ is wake capable.
Suggested-by?
...
> -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)
It's perfectly fine to replace ) by , on the previous line, no need
to make it shorter.
...
> +int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index)
> +{
> + struct resource r;
struct resource r = {};
?
> + return fwnode_irq_get_resource(fwnode, index, &r);
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 20/22] device property: Modify fwnode irq_get() to use resource
2023-12-21 13:56 ` Andy Shevchenko
@ 2023-12-21 23:46 ` Mark Hasemeyer
2023-12-22 12:44 ` Andy Shevchenko
[not found] ` <CAHQZ30BOA7zuRrN-kK5Qw+NYSVydfhJ0gDPr9q-U+7VKXHzG8g@mail.gmail.com>
0 siblings, 2 replies; 20+ messages in thread
From: Mark Hasemeyer @ 2023-12-21 23:46 UTC (permalink / raw)
To: Andy Shevchenko
Cc: LKML, AngeloGioacchino Del Regno, Krzysztof Kozlowski,
Tzung-Bi Shih, Raul Rangel, Konrad Dybcio, Rob Herring,
Sudeep Holla, Daniel Scally, Frank Rowand, Greg Kroah-Hartman,
Heikki Krogerus, Len Brown, Rafael J. Wysocki, Rob Herring,
Sakari Ailus, devicetree, linux-acpi
> > -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)
>
> It's perfectly fine to replace ) by , on the previous line, no need
> to make it shorter.
That puts the line at 115 chars? checkpatch.pl allows a maximum line
length of 100. I can bump the 'index' argument up a line and keep it
to a length of 95?
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 20/22] device property: Modify fwnode irq_get() to use resource
2023-12-21 23:46 ` Mark Hasemeyer
@ 2023-12-22 12:44 ` Andy Shevchenko
[not found] ` <CAHQZ30BOA7zuRrN-kK5Qw+NYSVydfhJ0gDPr9q-U+7VKXHzG8g@mail.gmail.com>
1 sibling, 0 replies; 20+ messages in thread
From: Andy Shevchenko @ 2023-12-22 12:44 UTC (permalink / raw)
To: Mark Hasemeyer
Cc: LKML, AngeloGioacchino Del Regno, Krzysztof Kozlowski,
Tzung-Bi Shih, Raul Rangel, Konrad Dybcio, Rob Herring,
Sudeep Holla, Daniel Scally, Frank Rowand, Greg Kroah-Hartman,
Heikki Krogerus, Len Brown, Rafael J. Wysocki, Rob Herring,
Sakari Ailus, devicetree, linux-acpi
On Thu, Dec 21, 2023 at 04:46:11PM -0700, Mark Hasemeyer wrote:
> > > -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)
> >
> > It's perfectly fine to replace ) by , on the previous line, no need
> > to make it shorter.
>
> That puts the line at 115 chars? checkpatch.pl allows a maximum line
> length of 100. I can bump the 'index' argument up a line and keep it
> to a length of 95?
Yes, the point is to leave index on the previous line and add a new one with
the r.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 20+ messages in thread[parent not found: <CAHQZ30BOA7zuRrN-kK5Qw+NYSVydfhJ0gDPr9q-U+7VKXHzG8g@mail.gmail.com>]
* Re: [PATCH v2 20/22] device property: Modify fwnode irq_get() to use resource
[not found] ` <CAHQZ30BOA7zuRrN-kK5Qw+NYSVydfhJ0gDPr9q-U+7VKXHzG8g@mail.gmail.com>
@ 2023-12-21 23:59 ` Mark Hasemeyer
2023-12-22 12:46 ` Andy Shevchenko
2023-12-22 12:46 ` Andy Shevchenko
1 sibling, 1 reply; 20+ messages in thread
From: Mark Hasemeyer @ 2023-12-21 23:59 UTC (permalink / raw)
To: Raul Rangel
Cc: Andy Shevchenko, LKML, AngeloGioacchino Del Regno,
Krzysztof Kozlowski, Tzung-Bi Shih, Konrad Dybcio, Rob Herring,
Sudeep Holla, Daniel Scally, Frank Rowand, Greg Kroah-Hartman,
Heikki Krogerus, Len Brown, Rafael J. Wysocki, Rob Herring,
Sakari Ailus, devicetree, linux-acpi
>> > > -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)
>> >
>> > It's perfectly fine to replace ) by , on the previous line, no need
>> > to make it shorter.
>>
>> That puts the line at 115 chars? checkpatch.pl allows a maximum line
>> length of 100. I can bump the 'index' argument up a line and keep it
>> to a length of 95?
>
>
> clang-format should do the right thing.
It formats the line as-is in the patch: with 'unsigned int index' on
the next line.
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 20/22] device property: Modify fwnode irq_get() to use resource
2023-12-21 23:59 ` Mark Hasemeyer
@ 2023-12-22 12:46 ` Andy Shevchenko
0 siblings, 0 replies; 20+ messages in thread
From: Andy Shevchenko @ 2023-12-22 12:46 UTC (permalink / raw)
To: Mark Hasemeyer
Cc: Raul Rangel, LKML, AngeloGioacchino Del Regno,
Krzysztof Kozlowski, Tzung-Bi Shih, Konrad Dybcio, Rob Herring,
Sudeep Holla, Daniel Scally, Frank Rowand, Greg Kroah-Hartman,
Heikki Krogerus, Len Brown, Rafael J. Wysocki, Rob Herring,
Sakari Ailus, devicetree, linux-acpi
On Thu, Dec 21, 2023 at 04:59:42PM -0700, Mark Hasemeyer wrote:
> >> > > -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)
> >> >
> >> > It's perfectly fine to replace ) by , on the previous line, no need
> >> > to make it shorter.
> >>
> >> That puts the line at 115 chars? checkpatch.pl allows a maximum line
> >> length of 100. I can bump the 'index' argument up a line and keep it
> >> to a length of 95?
> >
> > clang-format should do the right thing.
>
> It formats the line as-is in the patch: with 'unsigned int index' on
> the next line.
Exactly, and I don't think we need that "smartness" in this case.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 20/22] device property: Modify fwnode irq_get() to use resource
[not found] ` <CAHQZ30BOA7zuRrN-kK5Qw+NYSVydfhJ0gDPr9q-U+7VKXHzG8g@mail.gmail.com>
2023-12-21 23:59 ` Mark Hasemeyer
@ 2023-12-22 12:46 ` Andy Shevchenko
1 sibling, 0 replies; 20+ messages in thread
From: Andy Shevchenko @ 2023-12-22 12:46 UTC (permalink / raw)
To: Raul Rangel
Cc: Mark Hasemeyer, LKML, AngeloGioacchino Del Regno,
Krzysztof Kozlowski, Tzung-Bi Shih, Konrad Dybcio, Rob Herring,
Sudeep Holla, Daniel Scally, Frank Rowand, Greg Kroah-Hartman,
Heikki Krogerus, Len Brown, Rafael J. Wysocki, Rob Herring,
Sakari Ailus, devicetree, linux-acpi
On Thu, Dec 21, 2023 at 04:47:59PM -0700, Raul Rangel wrote:
> On Thu, Dec 21, 2023 at 4:46 PM Mark Hasemeyer <markhas@chromium.org> wrote:
>
> > > > -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)
> > >
> > > It's perfectly fine to replace ) by , on the previous line, no need
> > > to make it shorter.
> >
> > That puts the line at 115 chars? checkpatch.pl allows a maximum line
> > length of 100. I can bump the 'index' argument up a line and keep it
> > to a length of 95?
>
> clang-format should do the right thing.
Sorry, but no. It has some interesting results sometimes.
Like any other tool it has to be used with caution and
common sense. If it works in the particular case, fine.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 00/22] Improve IRQ wake capability reporting and update the cros_ec driver to use it
2023-12-20 23:54 [PATCH v2 00/22] Improve IRQ wake capability reporting and update the cros_ec driver to use it Mark Hasemeyer
` (2 preceding siblings ...)
2023-12-20 23:54 ` [PATCH v2 20/22] device property: Modify fwnode irq_get() " Mark Hasemeyer
@ 2023-12-21 13:42 ` Andy Shevchenko
2023-12-22 22:30 ` Mark Hasemeyer
3 siblings, 1 reply; 20+ messages in thread
From: Andy Shevchenko @ 2023-12-21 13:42 UTC (permalink / raw)
To: Mark Hasemeyer
Cc: LKML, 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 Wed, Dec 20, 2023 at 04:54:14PM -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.
>
> Some logic is added to the platform, ACPI, and DT subsystems to more
> easily pipe wakeirq information up to the driver.
Just wondering if you used --histogram diff algo when preparing patches.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 00/22] Improve IRQ wake capability reporting and update the cros_ec driver to use it
2023-12-21 13:42 ` [PATCH v2 00/22] Improve IRQ wake capability reporting and update the cros_ec driver to use it Andy Shevchenko
@ 2023-12-22 22:30 ` Mark Hasemeyer
2023-12-27 16:01 ` Andy Shevchenko
0 siblings, 1 reply; 20+ messages in thread
From: Mark Hasemeyer @ 2023-12-22 22:30 UTC (permalink / raw)
To: Andy Shevchenko
Cc: LKML, 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
> Just wondering if you used --histogram diff algo when preparing patches.
Not knowingly. I use patman which uses 'git format-patch' under the
covers with some added options:
https://github.com/siemens/u-boot/blob/master/tools/patman/gitutil.py#L308
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 00/22] Improve IRQ wake capability reporting and update the cros_ec driver to use it
2023-12-22 22:30 ` Mark Hasemeyer
@ 2023-12-27 16:01 ` Andy Shevchenko
0 siblings, 0 replies; 20+ messages in thread
From: Andy Shevchenko @ 2023-12-27 16:01 UTC (permalink / raw)
To: Mark Hasemeyer
Cc: LKML, 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 Fri, Dec 22, 2023 at 03:30:43PM -0700, Mark Hasemeyer wrote:
> > Just wondering if you used --histogram diff algo when preparing patches.
>
> Not knowingly. I use patman which uses 'git format-patch' under the
> covers with some added options:
> https://github.com/siemens/u-boot/blob/master/tools/patman/gitutil.py#L308
Add a configuration into your ~/.gitconfig (or local for the project),
it really makes the difference.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 20+ messages in thread