linux-kernel - Re: i915 hibernation patch (was: Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LFD.1.00.0802221653000.21332@woody.linux-foundation.org>
Date:	Fri, 22 Feb 2008 17:00:06 -0800 (PST)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	"Rafael J. Wysocki" <rjw@...k.pl>
cc:	Jesse Barnes <jesse.barnes@...el.com>,
	Jeff Chua <jeff.chua.linux@...il.com>,
	Romano Giannetti <romano@....icai.upcomillas.es>,
	Dave Airlie <airlied@...ux.ie>, Greg KH <gregkh@...e.de>,
	lkml <linux-kernel@...r.kernel.org>, linux-acpi@...r.kernel.org,
	Pavel Machek <pavel@....cz>,
	Alexey Starikovskiy <astarikovskiy@...e.de>,
	pm list <linux-pm@...ts.linux-foundation.org>
Subject: Re: i915 hibernation patch (was: Re: [Suspend-devel] 2.6.25-rc2
 System no longer powers off aftersuspend-to-disk. Screen becomes green.)



On Sat, 23 Feb 2008, Rafael J. Wysocki wrote:

> --- linux-2.6.orig/drivers/char/drm/i915_drv.c
> +++ linux-2.6/drivers/char/drm/i915_drv.c
> @@ -222,6 +222,7 @@ static void i915_restore_vga(struct drm_
>  			   dev_priv->saveGR[0x18]);
>  
>  	/* Attribute controller registers */
> +	inb(st01);
>  	for (i = 0; i < 20; i++)
>  		i915_write_ar(st01, i, dev_priv->saveAR[i], 0);
>  	inb(st01); /* switch back to index mode */

I'm doing this part separately, please drop it - it has nothing to do with 
the rest of the patch.

I'd also suggest that you just add a helper function like

	int pm_event_powerdown(struct pm_message mesg)
	{
		return mesg.event >= PM_EVENT_SUSPEND;
	}

or something, so that you can have code like

	if (pm_event_powerdown(mesg))
		pci_set_power_state(pdev, PCI_D3hot);

instead of the test for EVENT_SUSPEND/HIBERNATE explicitly.

Of course, the places that already do a switch-statement are much better 
kept that way and just add PM_EVENT_HIBERNATE to the list.

> --- linux-2.6.orig/drivers/pci/pci.c
> +++ linux-2.6/drivers/pci/pci.c
> @@ -554,6 +554,7 @@ pci_power_t pci_choose_state(struct pci_
>  	case PM_EVENT_PRETHAW:
>  		/* REVISIT both freeze and pre-thaw "should" use D0 */
>  	case PM_EVENT_SUSPEND:
> +	case PM_EVENT_HIBERNATE:
>  		return PCI_D3hot;

Didn't you miss the apci_pci_choose_state() thing that also needs this 
extension?

> Index: linux-2.6/drivers/usb/host/u132-hcd.c
> ===================================================================
> --- linux-2.6.orig/drivers/usb/host/u132-hcd.c
> +++ linux-2.6/drivers/usb/host/u132-hcd.c
> @@ -3214,14 +3214,18 @@ static int u132_suspend(struct platform_
>                  return -ESHUTDOWN;
>          } else {
>                  int retval = 0;
> -                if (state.event == PM_EVENT_FREEZE) {
> +
> +		switch (state.event) {
> +		case PM_EVENT_FREEZE:
>                          retval = u132_bus_suspend(hcd);
> -                } else if (state.event == PM_EVENT_SUSPEND) {
> +			break;
> +		case PM_EVENT_SUSPEND:
> +		case PM_EVENT_HIBERNATE:
>                          int ports = MAX_U132_PORTS;
>                          while (ports-- > 0) {
>                                  port_power(u132, ports, 0);
>                          }
> -                }
> +			break;
>                  if (retval == 0)
>                          pdev->dev.power.power_state = state;
>                  return retval;

Looks like a missing close-brace to me there - you removed the final '}'.

Or am I blind?

Apart from those issues it looks fine to me.

			Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/