Re: [PATCH] ACPI / PM: allow deeper wakeup power states with no _SxD nor _SxW

[kbuild test robot]

Hi Daniel,

I love your patch! Perhaps something to improve:

[auto build test WARNING on v4.16-rc4]
[also build test WARNING on next-20180319]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Daniel-Drake/ACPI-PM-allow-deeper-wakeup-power-states-with-no-_SxD-nor-_SxW/20180319-185209
config: i386-randconfig-x074-201811 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/acpi/device_pm.c: In function 'acpi_dev_pm_get_state':
>> drivers/acpi/device_pm.c:607:7: warning: 'sxd_status' may be used uninitialized in this function [-Wmaybe-uninitialized]
       if (sxd_status == AE_OK && target_state > ACPI_STATE_S0)
          ^

vim +/sxd_status +607 drivers/acpi/device_pm.c

   516	
   517	/**
   518	 * acpi_dev_pm_get_state - Get preferred power state of ACPI device.
   519	 * @dev: Device whose preferred target power state to return.
   520	 * @adev: ACPI device node corresponding to @dev.
   521	 * @target_state: System state to match the resultant device state.
   522	 * @d_min_p: Location to store the highest power state available to the device.
   523	 * @d_max_p: Location to store the lowest power state available to the device.
   524	 *
   525	 * Find the lowest power (highest number) and highest power (lowest number) ACPI
   526	 * device power states that the device can be in while the system is in the
   527	 * state represented by @target_state.  Store the integer numbers representing
   528	 * those stats in the memory locations pointed to by @d_max_p and @d_min_p,
   529	 * respectively.
   530	 *
   531	 * Callers must ensure that @dev and @adev are valid pointers and that @adev
   532	 * actually corresponds to @dev before using this function.
   533	 *
   534	 * Returns 0 on success or -ENODATA when one of the ACPI methods fails or
   535	 * returns a value that doesn't make sense.  The memory locations pointed to by
   536	 * @d_max_p and @d_min_p are only modified on success.
   537	 */
   538	static int acpi_dev_pm_get_state(struct device *dev, struct acpi_device *adev,
   539					 u32 target_state, int *d_min_p, int *d_max_p)
   540	{
   541		char method[] = { '_', 'S', '0' + target_state, 'D', '\0' };
   542		acpi_handle handle = adev->handle;
   543		unsigned long long ret;
   544		int d_min, d_max;
   545		bool wakeup = false;
   546		acpi_status sxd_status;
   547		acpi_status status;
   548	
   549		/*
   550		 * If the system state is S0, the lowest power state the device can be
   551		 * in is D3cold, unless the device has _S0W and is supposed to signal
   552		 * wakeup, in which case the return value of _S0W has to be used as the
   553		 * lowest power state available to the device.
   554		 */
   555		d_min = ACPI_STATE_D0;
   556		d_max = ACPI_STATE_D3_COLD;
   557	
   558		/*
   559		 * If present, _SxD methods return the minimum D-state (highest power
   560		 * state) we can use for the corresponding S-states.  Otherwise, the
   561		 * minimum D-state is D0 (ACPI 3.x).
   562		 */
   563		if (target_state > ACPI_STATE_S0) {
   564			/*
   565			 * We rely on acpi_evaluate_integer() not clobbering the integer
   566			 * provided if AE_NOT_FOUND is returned.
   567			 */
   568			ret = d_min;
   569			sxd_status = acpi_evaluate_integer(handle, method, NULL, &ret);
   570			if ((ACPI_FAILURE(sxd_status) && sxd_status != AE_NOT_FOUND)
   571			    || ret > ACPI_STATE_D3_COLD)
   572				return -ENODATA;
   573	
   574			/*
   575			 * We need to handle legacy systems where D3hot and D3cold are
   576			 * the same and 3 is returned in both cases, so fall back to
   577			 * D3cold if D3hot is not a valid state.
   578			 */
   579			if (!adev->power.states[ret].flags.valid) {
   580				if (ret == ACPI_STATE_D3_HOT)
   581					ret = ACPI_STATE_D3_COLD;
   582				else
   583					return -ENODATA;
   584			}
   585			d_min = ret;
   586			wakeup = device_may_wakeup(dev) && adev->wakeup.flags.valid
   587				&& adev->wakeup.sleep_state >= target_state;
   588		} else {
   589			wakeup = adev->wakeup.flags.valid;
   590		}
   591	
   592		/*
   593		 * If _PRW says we can wake up the system from the target sleep state,
   594		 * the D-state returned by _SxD is sufficient for that (we assume a
   595		 * wakeup-aware driver if wake is set).  Still, if _SxW exists
   596		 * (ACPI 3.x), it should return the maximum (lowest power) D-state that
   597		 * can wake the system.  _S0W may be valid, too.
   598		 */
   599		if (wakeup) {
   600			method[3] = 'W';
   601			status = acpi_evaluate_integer(handle, method, NULL, &ret);
   602			if (status == AE_NOT_FOUND) {
   603				/* No _SxW. In this case, the ACPI spec says that we
   604				 * must not go into any power state deeper than the
   605				 * value returned from _SxD.
   606				 */
 > 607				if (sxd_status == AE_OK && target_state > ACPI_STATE_S0)
   608					d_max = d_min;
   609			} else if (ACPI_SUCCESS(status) && ret <= ACPI_STATE_D3_COLD) {
   610				/* Fall back to D3cold if ret is not a valid state. */
   611				if (!adev->power.states[ret].flags.valid)
   612					ret = ACPI_STATE_D3_COLD;
   613	
   614				d_max = ret > d_min ? ret : d_min;
   615			} else {
   616				return -ENODATA;
   617			}
   618		}
   619	
   620		if (d_min_p)
   621			*d_min_p = d_min;
   622	
   623		if (d_max_p)
   624			*d_max_p = d_max;
   625	
   626		return 0;
   627	}
   628	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Attachment: .config.gz
Description: application/gzip