[coreboot] [PATCH] Enable Suspend-to-RAM code based on config option

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Wed Feb 13 01:02:34 CET 2008


The patch is correct if you compare it with latest v3 code, but
incorrect if you compare it to v2 because v3 has inverted logic. See below.

Ron? This one is for you.

On 12.02.2008 19:26, Carl-Daniel Hailfinger wrote:
> Enable Suspend-to-RAM code based on Kconfig option.
> Does not work yet, so the Kconfig option depends on BROKEN.
>
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
>
> Index: LinuxBIOSv3-if0crud/southbridge/amd/cs5536/cs5536.c
> ===================================================================
> --- LinuxBIOSv3-if0crud/southbridge/amd/cs5536/cs5536.c	(Revision 587)
> +++ LinuxBIOSv3-if0crud/southbridge/amd/cs5536/cs5536.c	(Arbeitskopie)
> @@ -488,7 +488,7 @@
>  	}
>  	sb = (struct southbridge_amd_cs5536_dts_config *)dev->device_configuration;
>  
> -#if 0
> +#ifdef CONFIG_SUSPEND_TO_RAM
>  	if (!IsS3Resume())
>  	{
>  		struct acpi_init *aci = acpi_init_table;
>   

Looking at the equivalent v2 code and the history of these lines in v3,
I found a logic inversion committed in
> r385 | rminnich | 2007-06-27 22:52:49 +0200 (Mi, 27 Jun 2007) | 9 lines
>
> Trivial fixes for some typos, and a major fix for an unitialized
> variable.
>
> Add a license to dts.
>
> Signed-off-by: Ronald G. Minnich <rminnich at gmail.com>
> Acked-by: Stefan Reinauer <stepan at coresystems.de>
>  void chipsetinit(void)
>  {
>  	struct device *dev;
>  	msr_t msr;
>  	u32 msrnum;
> -	struct southbridge_amd_cs5536_config *sb =
> -	    (struct southbridge_amd_cs5536_config *)dev->device_configuration;
> +	struct southbridge_amd_cs5536_config *sb;
>  	struct msrinit *csi;
>  
>  	post_code(P80_CHIPSET_INIT);
> -
> +	dev = dev_find_device(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CS5536_ISA, 0);
> +	if (! dev) {
> +		printk(BIOS_ERR, "%s: Could not find the south bridge!\n", __FUNCTION));
> +		return;
> +	}
> +	sb = (struct southbridge_amd_cs5536_config *)dev->device_configuration;
> -	/* we hope NEVER to be in linuxbios when S3 resumes
> +#if 0
>  	   if (! IsS3Resume()) */

In the old (before r385) version, the code below was always executed.
In the new version, the code below is never executed.
v2 behaves like the code before r385.

>  	{
>  		struct acpiinit *aci = acpi_init_table;
> @@ -531,6 +534,7 @@
>  
>  		pm_chipset_init();
>  	}
> +#endif
>  
>  	/* set hd IRQ */
>  	outl(GPIOL_2_SET, GPIO_IO_BASE + GPIOL_INPUT_ENABLE);

I believe we have to revert the semantic change.


Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/





More information about the coreboot mailing list