All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 1/1] driver core: platform: Make platform_get_irq_optional() optional
@ 2021-03-31 14:45 Andy Shevchenko
  2021-04-06 19:25 ` Guenter Roeck
  0 siblings, 1 reply; 4+ messages in thread
From: Andy Shevchenko @ 2021-03-31 14:45 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Uwe Kleine-König, linux-kernel
  Cc: Rafael J. Wysocki, Andy Shevchenko, Matthias Schiffer

Currently the platform_get_irq_optional() returns an error code even
if IRQ resource sumply has not been found. It prevents caller to be
error code agnostic in their error handling.

Now:
	ret = platform_get_irq_optional(...);
	if (ret != -ENXIO)
		return ret; // respect deferred probe
	if (ret > 0)
		...we get an IRQ...

After proposed change:
	ret = platform_get_irq_optional(...);
	if (ret < 0)
		return ret;
	if (ret > 0)
		...we get an IRQ...

Reported-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/platform.c | 55 +++++++++++++++++++++++++----------------
 1 file changed, 34 insertions(+), 21 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 6e1f8e0b661c..d82db3eabcd4 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -168,25 +168,7 @@ devm_platform_ioremap_resource_byname(struct platform_device *pdev,
 EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource_byname);
 #endif /* CONFIG_HAS_IOMEM */
 
-/**
- * platform_get_irq_optional - get an optional IRQ for a device
- * @dev: platform device
- * @num: IRQ number index
- *
- * Gets an IRQ for a platform device. Device drivers should check the return
- * value for errors so as to not pass a negative integer value to the
- * request_irq() APIs. This is the same as platform_get_irq(), except that it
- * does not print an error message if an IRQ can not be obtained.
- *
- * For example::
- *
- *		int irq = platform_get_irq_optional(pdev, 0);
- *		if (irq < 0)
- *			return irq;
- *
- * Return: non-zero IRQ number on success, negative error number on failure.
- */
-int platform_get_irq_optional(struct platform_device *dev, unsigned int num)
+static int platform_do_get_irq(struct platform_device *dev, unsigned int num)
 {
 	int ret;
 #ifdef CONFIG_SPARC
@@ -255,6 +237,37 @@ int platform_get_irq_optional(struct platform_device *dev, unsigned int num)
 	WARN(ret == 0, "0 is an invalid IRQ number\n");
 	return ret;
 }
+
+/**
+ * platform_get_irq_optional - get an optional IRQ for a device
+ * @dev: platform device
+ * @num: IRQ number index
+ *
+ * Gets an IRQ for a platform device. Device drivers should check the return
+ * value for errors so as to not pass a negative integer value to the
+ * request_irq() APIs. This is the same as platform_get_irq(), except that it
+ * does not print an error message if an IRQ can not be obtained and returns
+ * 0 when IRQ resource has not been found.
+ *
+ * For example::
+ *
+ *		int irq = platform_get_irq_optional(pdev, 0);
+ *		if (irq < 0)
+ *			return irq;
+ *		if (irq > 0)
+ *			...we have IRQ line defined...
+ *
+ * Return: non-zero IRQ number on success, negative error number on failure.
+ */
+int platform_get_irq_optional(struct platform_device *dev, unsigned int num)
+{
+	int ret;
+
+	ret = platform_do_get_irq(dev, num);
+	if (ret == -ENXIO)
+		return 0;
+	return ret;
+}
 EXPORT_SYMBOL_GPL(platform_get_irq_optional);
 
 /**
@@ -278,7 +291,7 @@ int platform_get_irq(struct platform_device *dev, unsigned int num)
 {
 	int ret;
 
-	ret = platform_get_irq_optional(dev, num);
+	ret = platform_do_get_irq(dev, num);
 	if (ret < 0 && ret != -EPROBE_DEFER)
 		dev_err(&dev->dev, "IRQ index %u not found\n", num);
 
@@ -296,7 +309,7 @@ int platform_irq_count(struct platform_device *dev)
 {
 	int ret, nr = 0;
 
-	while ((ret = platform_get_irq_optional(dev, nr)) >= 0)
+	while ((ret = platform_do_get_irq(dev, nr)) >= 0)
 		nr++;
 
 	if (ret == -EPROBE_DEFER)
-- 
2.30.2


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

* Re: [PATCH v1 1/1] driver core: platform: Make platform_get_irq_optional() optional
  2021-03-31 14:45 [PATCH v1 1/1] driver core: platform: Make platform_get_irq_optional() optional Andy Shevchenko
@ 2021-04-06 19:25 ` Guenter Roeck
  2021-04-07  9:38   ` Andy Shevchenko
  2021-04-07  9:49   ` Greg Kroah-Hartman
  0 siblings, 2 replies; 4+ messages in thread
From: Guenter Roeck @ 2021-04-06 19:25 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Greg Kroah-Hartman, Uwe Kleine-König, linux-kernel,
	Rafael J. Wysocki, Matthias Schiffer

On Wed, Mar 31, 2021 at 05:45:26PM +0300, Andy Shevchenko wrote:
> Currently the platform_get_irq_optional() returns an error code even
> if IRQ resource sumply has not been found. It prevents caller to be
> error code agnostic in their error handling.
> 
> Now:
> 	ret = platform_get_irq_optional(...);
> 	if (ret != -ENXIO)
> 		return ret; // respect deferred probe
> 	if (ret > 0)
> 		...we get an IRQ...
> 
> After proposed change:
> 	ret = platform_get_irq_optional(...);
> 	if (ret < 0)
> 		return ret;
> 	if (ret > 0)
> 		...we get an IRQ...
> 
> Reported-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

This patch causes all my "sh" emulations to stall during boot with the
following repeated error message.

sh-sci sh-sci.1: Can't allocate rx full IRQ

Reverting this patch fixes the problem (and the message is gone).
Bisect log is attached.

Guenter

---
# bad: [9c54130cd25528028b2d38f6ada0c79e92210390] Add linux-next specific files for 20210406
# good: [e49d033bddf5b565044e2abe4241353959bc9120] Linux 5.12-rc6
git bisect start 'HEAD' 'v5.12-rc6'
# good: [3981dcd7199773fc8cfbbcc6173e3521b8e49035] Merge remote-tracking branch 'crypto/master'
git bisect good 3981dcd7199773fc8cfbbcc6173e3521b8e49035
# good: [da18b6c82eba21e87d6ee76b7b0382eca123cc79] Merge remote-tracking branch 'ftrace/for-next'
git bisect good da18b6c82eba21e87d6ee76b7b0382eca123cc79
# bad: [021e2b99a3cb523408609ca1792ab623ff16f334] Merge remote-tracking branch 'staging/staging-next'
git bisect bad 021e2b99a3cb523408609ca1792ab623ff16f334
# bad: [685f903c62e3929370293bad184afa04b6fddebd] Merge remote-tracking branch 'char-misc/char-misc-next'
git bisect bad 685f903c62e3929370293bad184afa04b6fddebd
# good: [67d49fe7e4d40cfe6919b434d6a4e837230af9d4] Merge remote-tracking branch 'ipmi/for-next'
git bisect good 67d49fe7e4d40cfe6919b434d6a4e837230af9d4
# bad: [69e2ae87cfa94c77c3503715e9e0a68e6cc69f8d] Merge remote-tracking branch 'usb/usb-next'
git bisect bad 69e2ae87cfa94c77c3503715e9e0a68e6cc69f8d
# good: [2665a13a3e9ef3d08b9ac4b48328ddfba9126987] usb: typec: Fix a typo
git bisect good 2665a13a3e9ef3d08b9ac4b48328ddfba9126987
# good: [967f6d162d9fa415cf140d3eef5576d566632292] dt-bindings: usb: mtk-xhci: remove redefinitions of usb3-lpm-capable
git bisect good 967f6d162d9fa415cf140d3eef5576d566632292
# good: [d225ef6fda7ce9ff7d28764bd1cceea2d0215e8b] base: dd: fix error return code of driver_sysfs_add()
git bisect good d225ef6fda7ce9ff7d28764bd1cceea2d0215e8b
# bad: [72a91f192da032b68519fafaecce03fd002d669a] driver core: add helper for deferred probe reason setting
git bisect bad 72a91f192da032b68519fafaecce03fd002d669a
# good: [1768289b44bae847612751d418fc5c5e680b5e5c] driver core: platform: Declare early_platform_cleanup() prototype
git bisect good 1768289b44bae847612751d418fc5c5e680b5e5c
# bad: [ed7027fdf4ec41ed6df6814956dc11860232a9d5] driver core: platform: Make platform_get_irq_optional() optional
git bisect bad ed7027fdf4ec41ed6df6814956dc11860232a9d5
# good: [318c3e00f13c2f6e11202a22cc302ea8c70552ea] driver core: Replace printf() specifier and drop unneeded casting
git bisect good 318c3e00f13c2f6e11202a22cc302ea8c70552ea
# first bad commit: [ed7027fdf4ec41ed6df6814956dc11860232a9d5] driver core: platform: Make platform_get_irq_optional() optional

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

* Re: [PATCH v1 1/1] driver core: platform: Make platform_get_irq_optional() optional
  2021-04-06 19:25 ` Guenter Roeck
@ 2021-04-07  9:38   ` Andy Shevchenko
  2021-04-07  9:49   ` Greg Kroah-Hartman
  1 sibling, 0 replies; 4+ messages in thread
From: Andy Shevchenko @ 2021-04-07  9:38 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Greg Kroah-Hartman, Uwe Kleine-König, linux-kernel,
	Rafael J. Wysocki, Matthias Schiffer

On Tue, Apr 06, 2021 at 12:25:14PM -0700, Guenter Roeck wrote:
> On Wed, Mar 31, 2021 at 05:45:26PM +0300, Andy Shevchenko wrote:
> > Currently the platform_get_irq_optional() returns an error code even
> > if IRQ resource sumply has not been found. It prevents caller to be
> > error code agnostic in their error handling.
> > 
> > Now:
> > 	ret = platform_get_irq_optional(...);
> > 	if (ret != -ENXIO)
> > 		return ret; // respect deferred probe
> > 	if (ret > 0)
> > 		...we get an IRQ...
> > 
> > After proposed change:
> > 	ret = platform_get_irq_optional(...);
> > 	if (ret < 0)
> > 		return ret;
> > 	if (ret > 0)
> > 		...we get an IRQ...
> > 
> > Reported-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> This patch causes all my "sh" emulations to stall during boot with the
> following repeated error message.
> 
> sh-sci sh-sci.1: Can't allocate rx full IRQ
> 
> Reverting this patch fixes the problem (and the message is gone).
> Bisect log is attached.

I believe it reveals some "interesting" error handling there.
I'm going to propose a solution soon.

Thanks for the report!

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v1 1/1] driver core: platform: Make platform_get_irq_optional() optional
  2021-04-06 19:25 ` Guenter Roeck
  2021-04-07  9:38   ` Andy Shevchenko
@ 2021-04-07  9:49   ` Greg Kroah-Hartman
  1 sibling, 0 replies; 4+ messages in thread
From: Greg Kroah-Hartman @ 2021-04-07  9:49 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Andy Shevchenko, Uwe Kleine-König, linux-kernel,
	Rafael J. Wysocki, Matthias Schiffer

On Tue, Apr 06, 2021 at 12:25:14PM -0700, Guenter Roeck wrote:
> On Wed, Mar 31, 2021 at 05:45:26PM +0300, Andy Shevchenko wrote:
> > Currently the platform_get_irq_optional() returns an error code even
> > if IRQ resource sumply has not been found. It prevents caller to be
> > error code agnostic in their error handling.
> > 
> > Now:
> > 	ret = platform_get_irq_optional(...);
> > 	if (ret != -ENXIO)
> > 		return ret; // respect deferred probe
> > 	if (ret > 0)
> > 		...we get an IRQ...
> > 
> > After proposed change:
> > 	ret = platform_get_irq_optional(...);
> > 	if (ret < 0)
> > 		return ret;
> > 	if (ret > 0)
> > 		...we get an IRQ...
> > 
> > Reported-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> This patch causes all my "sh" emulations to stall during boot with the
> following repeated error message.
> 
> sh-sci sh-sci.1: Can't allocate rx full IRQ
> 
> Reverting this patch fixes the problem (and the message is gone).
> Bisect log is attached.
> 
> Guenter
> 
> ---
> # bad: [9c54130cd25528028b2d38f6ada0c79e92210390] Add linux-next specific files for 20210406
> # good: [e49d033bddf5b565044e2abe4241353959bc9120] Linux 5.12-rc6
> git bisect start 'HEAD' 'v5.12-rc6'
> # good: [3981dcd7199773fc8cfbbcc6173e3521b8e49035] Merge remote-tracking branch 'crypto/master'
> git bisect good 3981dcd7199773fc8cfbbcc6173e3521b8e49035
> # good: [da18b6c82eba21e87d6ee76b7b0382eca123cc79] Merge remote-tracking branch 'ftrace/for-next'
> git bisect good da18b6c82eba21e87d6ee76b7b0382eca123cc79
> # bad: [021e2b99a3cb523408609ca1792ab623ff16f334] Merge remote-tracking branch 'staging/staging-next'
> git bisect bad 021e2b99a3cb523408609ca1792ab623ff16f334
> # bad: [685f903c62e3929370293bad184afa04b6fddebd] Merge remote-tracking branch 'char-misc/char-misc-next'
> git bisect bad 685f903c62e3929370293bad184afa04b6fddebd
> # good: [67d49fe7e4d40cfe6919b434d6a4e837230af9d4] Merge remote-tracking branch 'ipmi/for-next'
> git bisect good 67d49fe7e4d40cfe6919b434d6a4e837230af9d4
> # bad: [69e2ae87cfa94c77c3503715e9e0a68e6cc69f8d] Merge remote-tracking branch 'usb/usb-next'
> git bisect bad 69e2ae87cfa94c77c3503715e9e0a68e6cc69f8d
> # good: [2665a13a3e9ef3d08b9ac4b48328ddfba9126987] usb: typec: Fix a typo
> git bisect good 2665a13a3e9ef3d08b9ac4b48328ddfba9126987
> # good: [967f6d162d9fa415cf140d3eef5576d566632292] dt-bindings: usb: mtk-xhci: remove redefinitions of usb3-lpm-capable
> git bisect good 967f6d162d9fa415cf140d3eef5576d566632292
> # good: [d225ef6fda7ce9ff7d28764bd1cceea2d0215e8b] base: dd: fix error return code of driver_sysfs_add()
> git bisect good d225ef6fda7ce9ff7d28764bd1cceea2d0215e8b
> # bad: [72a91f192da032b68519fafaecce03fd002d669a] driver core: add helper for deferred probe reason setting
> git bisect bad 72a91f192da032b68519fafaecce03fd002d669a
> # good: [1768289b44bae847612751d418fc5c5e680b5e5c] driver core: platform: Declare early_platform_cleanup() prototype
> git bisect good 1768289b44bae847612751d418fc5c5e680b5e5c
> # bad: [ed7027fdf4ec41ed6df6814956dc11860232a9d5] driver core: platform: Make platform_get_irq_optional() optional
> git bisect bad ed7027fdf4ec41ed6df6814956dc11860232a9d5
> # good: [318c3e00f13c2f6e11202a22cc302ea8c70552ea] driver core: Replace printf() specifier and drop unneeded casting
> git bisect good 318c3e00f13c2f6e11202a22cc302ea8c70552ea
> # first bad commit: [ed7027fdf4ec41ed6df6814956dc11860232a9d5] driver core: platform: Make platform_get_irq_optional() optional

Thanks for the report, I'll go revert this from my tree now.

greg k-h

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

end of thread, other threads:[~2021-04-07  9:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-31 14:45 [PATCH v1 1/1] driver core: platform: Make platform_get_irq_optional() optional Andy Shevchenko
2021-04-06 19:25 ` Guenter Roeck
2021-04-07  9:38   ` Andy Shevchenko
2021-04-07  9:49   ` Greg Kroah-Hartman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.