[Intel-gfx] [PATCH i-g-t v2 0/3] Cold Reset IGT Test

[Intel-gfx] [PATCH i-g-t v2 0/3] Cold Reset IGT Test

[Anshuman Gupta]

Cold Reset IGT Test.

v2:
Address some review comments from Kamil and Badal.

Anshuman Gupta (3):
  lib/igt_pm: Refactor get firmware_node fd
  test/device_reset: Refactor initiate_device_reset
  tests/device_reset: Add cold reset IGT test

 lib/igt_pm.c         |  76 ++++++++++++++++++++----
 lib/igt_pm.h         |   1 +
 tests/device_reset.c | 138 +++++++++++++++++++++++++++++++++++++++----
 3 files changed, 191 insertions(+), 24 deletions(-)

-- 
2.25.1

[Intel-gfx] [PATCH i-g-t v2 1/3] lib/igt_pm: Refactor get firmware_node fd

[Anshuman Gupta]

Created igt_pm_open_pci_firmware_node() to refactor
the retrieving the firmware_node fd code.

igt_pm_open_pci_firmware_node() will be used by other
firmware_node consumers.

While doing that fixed the leaked fd as well.

v2:
- return false instead of igt_require on no firmware_node_fd. [Kamil]
- use igt_assert() when failed to open 'real_power_state' on error
  other then ENONET.

Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 lib/igt_pm.c | 36 +++++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/lib/igt_pm.c b/lib/igt_pm.c
index 1e6e9ed3f..4f0cfbdd1 100644
--- a/lib/igt_pm.c
+++ b/lib/igt_pm.c
@@ -863,6 +863,20 @@ bool i915_output_is_lpsp_capable(int drm_fd, igt_output_t *output)
 	return strstr(buf, "LPSP: capable");
 }
 
+static int igt_pm_open_pci_firmware_node(struct pci_device *pci_dev)
+{
+	char name[PATH_MAX];
+	int dir;
+
+	snprintf(name, PATH_MAX,
+		 "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/firmware_node",
+		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
+
+	dir = open(name, O_RDONLY);
+
+	return dir;
+}
+
 /**
  * igt_pm_acpi_d3cold_supported:
  * @pci_dev: root port pci_dev.
@@ -873,23 +887,23 @@ bool i915_output_is_lpsp_capable(int drm_fd, igt_output_t *output)
  */
 bool igt_pm_acpi_d3cold_supported(struct pci_device *pci_dev)
 {
-	char name[PATH_MAX];
-	int dir, fd;
-
-	snprintf(name, PATH_MAX,
-		 "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/firmware_node",
-		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
+	int firmware_node_fd, fd;
 
-	dir = open(name, O_RDONLY);
-	igt_require(dir > 0);
+	firmware_node_fd = igt_pm_open_pci_firmware_node(pci_dev);
+	if (firmware_node_fd < 0)
+		return false;
 
 	/* BIOS need to enable ACPI D3Cold Support.*/
-	fd = openat(dir, "real_power_state", O_RDONLY);
-	if (fd < 0 && errno == ENOENT)
+	fd = openat(firmware_node_fd, "real_power_state", O_RDONLY);
+	if (fd < 0 && errno == ENOENT) {
+		close(firmware_node_fd);
 		return false;
+	}
 
-	igt_require(fd > 0);
+	igt_assert(errno > 0);
 
+	close(firmware_node_fd);
+	close(fd);
 	return true;
 }
 
-- 
2.25.1

[Intel-gfx] [PATCH i-g-t v2 2/3] test/device_reset: Refactor initiate_device_reset

[Anshuman Gupta]

Added a reset type enum to support multiple types
of reset like WARM, COLD and FLR reset.

No functional change.

v2:
- Removed WARM_RESET enum as not used yet. [Badal]

Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 tests/device_reset.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/tests/device_reset.c b/tests/device_reset.c
index e60d4c7fd..0c477a02c 100644
--- a/tests/device_reset.c
+++ b/tests/device_reset.c
@@ -19,6 +19,11 @@ IGT_TEST_DESCRIPTION("Examine behavior of a driver on device sysfs reset");
 #define DEV_PATH_LEN 80
 #define DEV_BUS_ADDR_LEN 13 /* addr has form 0000:00:00.0 */
 
+enum reset {
+	COLD_RESET,
+	FLR_RESET
+};
+
 /**
  * Helper structure containing file descriptors
  * and bus address related to tested device
@@ -222,10 +227,13 @@ static void driver_bind(struct device_fds *dev)
 }
 
 /* Initiate device reset */
-static void initiate_device_reset(struct device_fds *dev)
+static void initiate_device_reset(struct device_fds *dev, enum reset type)
 {
 	igt_debug("reset device\n");
-	igt_assert(igt_sysfs_set(dev->fds.dev_dir, "reset", "1"));
+
+	if (type == FLR_RESET)
+		igt_assert(igt_sysfs_set(dev->fds.dev_dir, "reset", "1"));
+
 }
 
 static bool is_i915_wedged(int i915)
@@ -274,14 +282,14 @@ static void set_device_filter(const char* dev_path)
 	igt_assert_eq(igt_device_filter_add(filter), 1);
 }
 
-static void unbind_reset_rebind(struct device_fds *dev)
+static void unbind_reset_rebind(struct device_fds *dev, enum reset type)
 {
 	igt_debug("close the device\n");
 	close_if_opened(&dev->fds.dev);
 
 	driver_unbind(dev);
 
-	initiate_device_reset(dev);
+	initiate_device_reset(dev, type);
 
 	driver_bind(dev);
 }
@@ -306,13 +314,13 @@ igt_main
 	igt_describe("Unbinds driver from device, initiates reset"
 		     " then rebinds driver to device");
 	igt_subtest("unbind-reset-rebind") {
-		unbind_reset_rebind(&dev);
+		unbind_reset_rebind(&dev, FLR_RESET);
 		healthcheck(&dev);
 	}
 
 	igt_describe("Resets device with bound driver");
 	igt_subtest("reset-bound") {
-		initiate_device_reset(&dev);
+		initiate_device_reset(&dev, FLR_RESET);
 		healthcheck(&dev);
 	}
 
-- 
2.25.1

[Intel-gfx] [PATCH i-g-t v2 3/3] tests/device_reset: Add cold reset IGT test

[Anshuman Gupta]

Add cold-reset IGT subtest, IGT subtest will use
/sys/bus/pci/slots/$SUN/power sysfs in order to
initiate a cold reset sequence. $SUN value will be
retrieved from PCIe device ACPI firmware node.

v2:
- %s/igt_require(fd > 0)/igt_assert(errno >0)

Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
Reviewed-by: Badal Nilawar <badal.nilawar@intel.com>
---
 lib/igt_pm.c         |  40 ++++++++++++++
 lib/igt_pm.h         |   1 +
 tests/device_reset.c | 124 +++++++++++++++++++++++++++++++++++++++----
 3 files changed, 155 insertions(+), 10 deletions(-)

diff --git a/lib/igt_pm.c b/lib/igt_pm.c
index 4f0cfbdd1..9b7ed4be6 100644
--- a/lib/igt_pm.c
+++ b/lib/igt_pm.c
@@ -877,6 +877,46 @@ static int igt_pm_open_pci_firmware_node(struct pci_device *pci_dev)
 	return dir;
 }
 
+/**
+ * igt_pm_get_pcie_acpihp_slot:
+ * @pci_dev: pci bridge device.
+ * Get pci bridge acpi hotplug slot number, if bridge's ACPI firmware_node
+ * handle supports _SUN method.
+ *
+ * Returns:
+ * PCIe bridge Slot number.
+ * Returns -ENOENT number in case firmware_node/sun is not supported by the
+ * bridge.
+ */
+int igt_pm_get_pcie_acpihp_slot(struct pci_device *pci_dev)
+{
+	int firmware_node_fd, fd, n_read;
+	char sun[8];
+
+	firmware_node_fd = igt_pm_open_pci_firmware_node(pci_dev);
+
+	if (firmware_node_fd < 0 && errno == ENOENT)
+		return -ENOENT;
+
+	igt_require(firmware_node_fd > 0);
+
+	fd = openat(firmware_node_fd, "sun", O_RDONLY);
+	if (fd < 0 && errno == ENOENT) {
+		close(firmware_node_fd);
+		return -ENOENT;
+	}
+
+	igt_assert(errno > 0);
+
+	n_read = read(fd, sun, sizeof(sun));
+	igt_assert(n_read > 0);
+
+	close(firmware_node_fd);
+	close(fd);
+
+	return strtol(sun, NULL, 10);
+}
+
 /**
  * igt_pm_acpi_d3cold_supported:
  * @pci_dev: root port pci_dev.
diff --git a/lib/igt_pm.h b/lib/igt_pm.h
index e81fceb89..f65b960c3 100644
--- a/lib/igt_pm.h
+++ b/lib/igt_pm.h
@@ -73,6 +73,7 @@ bool igt_wait_for_pm_status(enum igt_runtime_pm_status status);
 bool igt_pm_dmc_loaded(int debugfs);
 bool igt_pm_pc8_plus_residencies_enabled(int msr_fd);
 bool i915_output_is_lpsp_capable(int drm_fd, igt_output_t *output);
+int igt_pm_get_pcie_acpihp_slot(struct pci_device *pci_dev);
 bool igt_pm_acpi_d3cold_supported(struct pci_device *pci_dev);
 enum igt_acpi_d_state
 igt_pm_get_acpi_real_d_state(struct pci_device *pci_dev);
diff --git a/tests/device_reset.c b/tests/device_reset.c
index 0c477a02c..427ea0b09 100644
--- a/tests/device_reset.c
+++ b/tests/device_reset.c
@@ -9,6 +9,7 @@
 
 #include "i915/gem.h"
 #include "igt.h"
+#include "igt_device.h"
 #include "igt_device_scan.h"
 #include "igt_sysfs.h"
 #include "igt_kmod.h"
@@ -33,6 +34,7 @@ struct device_fds {
 		int dev;
 		int dev_dir;
 		int drv_dir;
+		int slot_dir; /* pci hotplug slots fd */
 	} fds;
 	char dev_bus_addr[DEV_BUS_ADDR_LEN];
 	bool snd_unload;
@@ -62,6 +64,45 @@ static int open_driver_sysfs_dir(int fd)
 	return __open_sysfs_dir(fd, "device/driver");
 }
 
+static int open_slot_sysfs_dir(int fd)
+{
+	struct pci_device *pci_dev = NULL;
+	int slot_fd = -1, slot;
+	char slot_fd_path[PATH_MAX];
+
+	pci_dev = igt_device_get_pci_device(fd);
+	igt_require(pci_dev);
+
+	while ((pci_dev = pci_device_get_parent_bridge(pci_dev))) {
+		slot = igt_pm_get_pcie_acpihp_slot(pci_dev);
+		if (slot == -ENOENT) {
+			igt_debug("Bridge PCI device %04x:%02x:%02x.%01x does not support acpihp slot\n",
+				  pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
+			continue;
+		}
+
+		/*
+		 * Upon getting the valid acpihp slot number break the loop.
+		 * It is the desired acpihp slot for gfx card.
+		 */
+		if (slot > 0) {
+			igt_debug("Bridge PCI device %04x:%02x:%02x.%01x associated acpihp slot %d\n",
+				  pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func, slot);
+			break;
+		}
+	}
+
+	if (!pci_dev)
+		return slot_fd;
+
+	snprintf(slot_fd_path, PATH_MAX, "/sys/bus/pci/slots/%d", slot);
+	slot_fd = open(slot_fd_path, O_RDONLY);
+	if (slot_fd < 0)
+		return -errno;
+
+	return slot_fd;
+}
+
 /**
  * device_sysfs_path:
  * @fd: opened device file descriptor
@@ -124,6 +165,8 @@ static void init_device_fds(struct device_fds *dev)
 
 	dev->fds.drv_dir = open_driver_sysfs_dir(dev->fds.dev);
 	igt_assert_fd(dev->fds.drv_dir);
+
+	dev->fds.slot_dir = open_slot_sysfs_dir(dev->fds.dev);
 }
 
 static int close_if_opened(int *fd)
@@ -142,6 +185,7 @@ static void cleanup_device_fds(struct device_fds *dev)
 	igt_ignore_warn(close_if_opened(&dev->fds.dev));
 	igt_ignore_warn(close_if_opened(&dev->fds.dev_dir));
 	igt_ignore_warn(close_if_opened(&dev->fds.drv_dir));
+	igt_ignore_warn(close_if_opened(&dev->fds.slot_dir));
 }
 
 /**
@@ -179,6 +223,34 @@ static bool is_sysfs_reset_supported(int fd)
 	return true;
 }
 
+/**
+ * is_sysfs_cold_reset_supported:
+ * @fd: opened device file descriptor
+ *
+ * Check if device supports cold reset based on sysfs file presence.
+ *
+ * Returns:
+ * True if device supports reset, false otherwise.
+ */
+static bool is_sysfs_cold_reset_supported(int slot_fd)
+{
+	struct stat st;
+	int rc;
+	int cold_reset_fd = -1;
+
+	cold_reset_fd = openat(slot_fd, "power", O_WRONLY);
+
+	if (cold_reset_fd < 0)
+		return false;
+
+	rc = fstat(cold_reset_fd, &st);
+	close(cold_reset_fd);
+
+	if (rc || !S_ISREG(st.st_mode))
+		return false;
+
+	return true;
+}
 /* Unbind the driver from the device */
 static void driver_unbind(struct device_fds *dev)
 {
@@ -231,8 +303,12 @@ static void initiate_device_reset(struct device_fds *dev, enum reset type)
 {
 	igt_debug("reset device\n");
 
-	if (type == FLR_RESET)
+	if (type == FLR_RESET) {
 		igt_assert(igt_sysfs_set(dev->fds.dev_dir, "reset", "1"));
+	} else if (type == COLD_RESET) {
+		igt_assert(igt_sysfs_set(dev->fds.slot_dir, "power", "0"));
+		igt_assert(igt_sysfs_set(dev->fds.slot_dir, "power", "1"));
+	}
 
 }
 
@@ -311,17 +387,45 @@ igt_main
 		igt_skip_on(!is_sysfs_reset_supported(dev.fds.dev));
 	}
 
-	igt_describe("Unbinds driver from device, initiates reset"
-		     " then rebinds driver to device");
-	igt_subtest("unbind-reset-rebind") {
-		unbind_reset_rebind(&dev, FLR_RESET);
-		healthcheck(&dev);
+	igt_subtest_group {
+		igt_describe("Unbinds driver from device, initiates reset"
+			     " then rebinds driver to device");
+		igt_subtest("unbind-reset-rebind") {
+			unbind_reset_rebind(&dev, FLR_RESET);
+			healthcheck(&dev);
+		}
+
+		igt_describe("Resets device with bound driver");
+		igt_subtest("reset-bound") {
+			initiate_device_reset(&dev, FLR_RESET);
+			/*
+			 * Cold reset will initiate card boot sequence again,
+			 * therefore let healthcheck() re-epen the dev fd.
+			 */
+			dev.fds.dev = -1;
+			healthcheck(&dev);
+		}
 	}
 
-	igt_describe("Resets device with bound driver");
-	igt_subtest("reset-bound") {
-		initiate_device_reset(&dev, FLR_RESET);
-		healthcheck(&dev);
+	igt_subtest_group {
+		igt_fixture {
+			igt_skip_on_f(dev.fds.slot_dir < 0, "Gfx Card does not support any "
+				      "pcie slot for cold reset\n");
+			igt_skip_on(!is_sysfs_cold_reset_supported(dev.fds.slot_dir));
+		}
+
+		igt_describe("Unbinds driver from device, initiates cold reset"
+			     " then rebinds driver to device");
+		igt_subtest("unbind-cold-reset-rebind") {
+			unbind_reset_rebind(&dev, COLD_RESET);
+			healthcheck(&dev);
+		}
+
+		igt_describe("Cold Resets device with bound driver");
+		igt_subtest("cold-reset-bound") {
+			initiate_device_reset(&dev, COLD_RESET);
+			healthcheck(&dev);
+		}
 	}
 
 	igt_fixture {
-- 
2.25.1

Re: [Intel-gfx] [PATCH i-g-t v2 1/3] lib/igt_pm: Refactor get firmware_node fd

[Rodrigo Vivi]

On Mon, Nov 14, 2022 at 06:08:41PM +0530, Anshuman Gupta wrote:
> Created igt_pm_open_pci_firmware_node() to refactor
> the retrieving the firmware_node fd code.
> 
> igt_pm_open_pci_firmware_node() will be used by other
> firmware_node consumers.
> 
> While doing that fixed the leaked fd as well.
> 
> v2:
> - return false instead of igt_require on no firmware_node_fd. [Kamil]
> - use igt_assert() when failed to open 'real_power_state' on error
>   other then ENONET.
> 
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  lib/igt_pm.c | 36 +++++++++++++++++++++++++-----------

I believe we need a massive refactor in this lib... we have 2 different ways
of using stuff, plus that ugly global restore...

but anyway, this one here looks a good change regardless of the current mess
in the lib.


Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>


>  1 file changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/igt_pm.c b/lib/igt_pm.c
> index 1e6e9ed3f..4f0cfbdd1 100644
> --- a/lib/igt_pm.c
> +++ b/lib/igt_pm.c
> @@ -863,6 +863,20 @@ bool i915_output_is_lpsp_capable(int drm_fd, igt_output_t *output)
>  	return strstr(buf, "LPSP: capable");
>  }
>  
> +static int igt_pm_open_pci_firmware_node(struct pci_device *pci_dev)
> +{
> +	char name[PATH_MAX];
> +	int dir;
> +
> +	snprintf(name, PATH_MAX,
> +		 "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/firmware_node",
> +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
> +
> +	dir = open(name, O_RDONLY);
> +
> +	return dir;
> +}
> +
>  /**
>   * igt_pm_acpi_d3cold_supported:
>   * @pci_dev: root port pci_dev.
> @@ -873,23 +887,23 @@ bool i915_output_is_lpsp_capable(int drm_fd, igt_output_t *output)
>   */
>  bool igt_pm_acpi_d3cold_supported(struct pci_device *pci_dev)
>  {
> -	char name[PATH_MAX];
> -	int dir, fd;
> -
> -	snprintf(name, PATH_MAX,
> -		 "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/firmware_node",
> -		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func);
> +	int firmware_node_fd, fd;
>  
> -	dir = open(name, O_RDONLY);
> -	igt_require(dir > 0);
> +	firmware_node_fd = igt_pm_open_pci_firmware_node(pci_dev);
> +	if (firmware_node_fd < 0)
> +		return false;
>  
>  	/* BIOS need to enable ACPI D3Cold Support.*/
> -	fd = openat(dir, "real_power_state", O_RDONLY);
> -	if (fd < 0 && errno == ENOENT)
> +	fd = openat(firmware_node_fd, "real_power_state", O_RDONLY);
> +	if (fd < 0 && errno == ENOENT) {
> +		close(firmware_node_fd);
>  		return false;
> +	}
>  
> -	igt_require(fd > 0);
> +	igt_assert(errno > 0);
>  
> +	close(firmware_node_fd);
> +	close(fd);
>  	return true;
>  }
>  
> -- 
> 2.25.1
> 

Re: [Intel-gfx] [PATCH i-g-t v2 1/3] lib/igt_pm: Refactor get firmware_node fd

[Gupta, Anshuman]

> -----Original Message-----
> From: Vivi, Rodrigo <rodrigo.vivi@intel.com>
> Sent: Tuesday, November 15, 2022 1:27 AM
> To: Gupta, Anshuman <anshuman.gupta@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; kamil.konieczny@linux.intel.com;
> Tangudu, Tilak <tilak.tangudu@intel.com>; Nilawar, Badal
> <badal.nilawar@intel.com>
> Subject: Re: [PATCH i-g-t v2 1/3] lib/igt_pm: Refactor get firmware_node fd
> 
> On Mon, Nov 14, 2022 at 06:08:41PM +0530, Anshuman Gupta wrote:
> > Created igt_pm_open_pci_firmware_node() to refactor the retrieving the
> > firmware_node fd code.
> >
> > igt_pm_open_pci_firmware_node() will be used by other firmware_node
> > consumers.
> >
> > While doing that fixed the leaked fd as well.
> >
> > v2:
> > - return false instead of igt_require on no firmware_node_fd. [Kamil]
> > - use igt_assert() when failed to open 'real_power_state' on error
> >   other then ENONET.
> >
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> >  lib/igt_pm.c | 36 +++++++++++++++++++++++++-----------
> 
> I believe we need a massive refactor in this lib... we have 2 different ways of
> using stuff, plus that ugly global restore...
> 
> but anyway, this one here looks a good change regardless of the current
> mess in the lib.
> 
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Thanks Rodrigo for RB, 
my bad I sent these patches on kernel mailing list instead of igt-dev.
I will keep your RB and re-float the patches to igt-dev.
Thanks,
Anshuman Gupta.
> 
> 
> >  1 file changed, 25 insertions(+), 11 deletions(-)
> >
> > diff --git a/lib/igt_pm.c b/lib/igt_pm.c index 1e6e9ed3f..4f0cfbdd1
> > 100644
> > --- a/lib/igt_pm.c
> > +++ b/lib/igt_pm.c
> > @@ -863,6 +863,20 @@ bool i915_output_is_lpsp_capable(int drm_fd,
> igt_output_t *output)
> >  	return strstr(buf, "LPSP: capable");  }
> >
> > +static int igt_pm_open_pci_firmware_node(struct pci_device *pci_dev)
> > +{
> > +	char name[PATH_MAX];
> > +	int dir;
> > +
> > +	snprintf(name, PATH_MAX,
> > +
> "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/firmware_node",
> > +		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev-
> >func);
> > +
> > +	dir = open(name, O_RDONLY);
> > +
> > +	return dir;
> > +}
> > +
> >  /**
> >   * igt_pm_acpi_d3cold_supported:
> >   * @pci_dev: root port pci_dev.
> > @@ -873,23 +887,23 @@ bool i915_output_is_lpsp_capable(int drm_fd,
> igt_output_t *output)
> >   */
> >  bool igt_pm_acpi_d3cold_supported(struct pci_device *pci_dev)  {
> > -	char name[PATH_MAX];
> > -	int dir, fd;
> > -
> > -	snprintf(name, PATH_MAX,
> > -
> "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/firmware_node",
> > -		 pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev-
> >func);
> > +	int firmware_node_fd, fd;
> >
> > -	dir = open(name, O_RDONLY);
> > -	igt_require(dir > 0);
> > +	firmware_node_fd = igt_pm_open_pci_firmware_node(pci_dev);
> > +	if (firmware_node_fd < 0)
> > +		return false;
> >
> >  	/* BIOS need to enable ACPI D3Cold Support.*/
> > -	fd = openat(dir, "real_power_state", O_RDONLY);
> > -	if (fd < 0 && errno == ENOENT)
> > +	fd = openat(firmware_node_fd, "real_power_state", O_RDONLY);
> > +	if (fd < 0 && errno == ENOENT) {
> > +		close(firmware_node_fd);
> >  		return false;
> > +	}
> >
> > -	igt_require(fd > 0);
> > +	igt_assert(errno > 0);
> >
> > +	close(firmware_node_fd);
> > +	close(fd);
> >  	return true;
> >  }
> >
> > --
> > 2.25.1
> >