#if 0 crud in v3

[Carl-Daniel Hailfinger]

Hi,

Ron's latest patch triggered me to look at how much dead code we already
have in v3. The situation is not bad, but we could use more commits
which add comments before #if 0 and #if 1, just to make sure the code is
not left in forever without known purpose.

Analysis of the current state follows:

./southbridge/amd/cs5536/cs5536.c
> void chipsetinit(void)
> {
>    struct device *dev;
>    struct msr msr;
>    struct southbridge_amd_cs5536_dts_config *sb;
>    const struct msrinit *csi;
>
> post_code(P80_CHIPSET_INIT);
>    dev = dev_find_pci_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_dts_config *)dev->device_configuration;
>
> #if 0

How about
#ifdef CONFIG_SUSPEND_TO_RAM
> if (!IsS3Resume())
>    {
>    	struct acpi_init *aci = acpi_init_table;
>    	for (/* Nothing */; aci->ioreg; aci++) {
>    		outl(aci->regdata, aci->ioreg);
>    		inl(aci->ioreg);
>    	}
>    	pm_chipset_init();
>    }
> #endif
>
> /* Set HD IRQ. */
>    outl(GPIOL_2_SET, GPIO_IO_BASE + GPIOL_INPUT_ENABLE);
>    outl(GPIOL_2_SET, GPIO_IO_BASE + GPIOL_IN_AUX1_SELECT);
> static void southbridge_init(struct device *dev)
> {
>    struct southbridge_amd_cs5536_dts_config *sb =
>        (struct southbridge_amd_cs5536_dts_config *)dev->device_configuration;
>
> /*
>     * struct device *gpiodev;
>     * unsigned short gpiobase = MDD_GPIO;
>     */
>
> printk(BIOS_ERR, "cs5536: %s\n", __FUNCTION__);
>
> setup_i8259();
>    lpc_init(sb);
>    uarts_init(sb);
>
> if (sb->enable_gpio_int_route) {
>    	vr_write((VRC_MISCELLANEOUS << 8) + PCI_INT_AB,
>    		 (sb->enable_gpio_int_route & 0xFFFF));
>    	vr_write((VRC_MISCELLANEOUS << 8) + PCI_INT_CD,
>    		 (sb->enable_gpio_int_route >> 16));
>    }
>
> printk(BIOS_ERR, "cs5536: %s: enable_ide_nand_flash is %d\n",
>           __FUNCTION__, sb->enable_ide_nand_flash);
>    if (sb->enable_ide_nand_flash == 1)
>    	enable_ide_nand_flash_header();
>
> enable_USB_port4(sb);
>
> if (sb->enable_ide)
>    	ide_init(dev);
>
> #warning Add back in unwanted VPCI support
> #if 0

Any reason not to remove this #if 0?
> /* Disable unwanted virtual PCI devices. */
>    for (i = 0; (i < MAX_UNWANTED_VPCI) && (0 != sb->unwanted_vpci[i]); i++) {
>    	printk(BIOS_DEBUG, "Disabling VPCI device: 0x%08X\n",
>    	       sb->unwanted_vpci[i]);
>    	outl(sb->unwanted_vpci[i] + 0x7C, 0xCF8);
>    	outl(0xDEADBEEF, 0xCFC);
>    }
> #endif
>    printk(BIOS_SPEW, "cs5536: %s() Exit\n", __FUNCTION__);
> }
>
> /**

./northbridge/amd/geodelx/raminit.c
> static void check_ddr_max(u8 dimm0, u8 dimm1)
> {
>    u8 spd_byte0, spd_byte1;
>    u16 speed;
>
> /* PC133 identifier */
>    spd_byte0 = spd_read_byte(dimm0, SPD_MIN_CYCLE_TIME_AT_CAS_MAX);
>    if (spd_byte0 == 0xFF)
>    	spd_byte0 = 0;
>    spd_byte1 = spd_read_byte(dimm1, SPD_MIN_CYCLE_TIME_AT_CAS_MAX);
>    if (spd_byte1 == 0xFF)
>    	spd_byte1 = 0;
>
> /* I don't think you need this check. */
> #if 0

We may need an opinion from someone familiar with the hardware details...
> if (spd_byte0 < 0xA0 || spd_byte0 < 0xA0) {
>    	printk(BIOS_EMERG, "DIMM overclocked. Check GeodeLink speed\n");
>    	post_code(POST_PLL_MEM_FAIL);
>    	hlt();
>    }
> #endif
>
> /* Use the slowest DIMM. */
>    if (spd_byte0 < spd_byte1)
>    	spd_byte0 = spd_byte1;
>
> /* Turn SPD ns time into MHz. Check what the asm does to this math. */
>    speed = 2 * ((10000 / (((spd_byte0 >> 4) * 10) + (spd_byte0 & 0x0F))));
>
> /* Current speed > max speed? */
>    if (geode_link_speed() > speed) {
>    	printk(BIOS_EMERG, "DIMM overclocked. Check GeodeLink speed\n");
>    	post_code(POST_PLL_MEM_FAIL);
>    	hlt();
>    }
> }
> void sdram_set_registers(void)
> {
>    struct msr msr;
>
> /* Set Timing Control */
>    msr = rdmsr(MC_CF1017_DATA);
>    msr.lo &= ~(7 << CF1017_LOWER_RD_TMG_CTL_SHIFT);
>    if (geode_link_speed() < 334)
>    	msr.lo |= (3 << CF1017_LOWER_RD_TMG_CTL_SHIFT);
>    else
>    	msr.lo |= (4 << CF1017_LOWER_RD_TMG_CTL_SHIFT);
>    wrmsr(MC_CF1017_DATA, msr);
>
> /* Set Refresh Staggering */
>    msr = rdmsr(MC_CF07_DATA);
>    msr.lo &= ~0xF0;
>    msr.lo |= 0x40;		/* Set refresh to 4 SDRAM clocks. */
>    wrmsr(MC_CF07_DATA, msr);
>
> /* Memory Interleave: Set HOI here otherwise default is LOI. */
> #if 0

Switch to config option or NVRAM option or kill the code altogether?
> msr = rdmsr(MC_CF8F_DATA);
>    msr.hi |= CF8F_UPPER_HOI_LOI_SET;
>    wrmsr(MC_CF8F_DATA, msr);
> #endif
> }

./northbridge/amd/geodelx/geodelx.c
> static void geodelx_northbridge_init(struct device *dev)
> {
>    /* struct msr msr; */
>
> printk(BIOS_SPEW, ">> Entering northbridge.c: %s\n", __FUNCTION__);
>
> enable_shadow(dev);
>
> #if 0

No idea about this one.
> /* Swiss cheese */
>    msr = rdmsr(MSR_GLIU0_SHADOW);
>
> msr.hi |= 0x3;
>    msr.lo |= 0x30000;
>
> printk(BIOS_DEBUG, "MSR 0x%08X is now 0x%08X:0x%08X\n",
>           MSR_GLIU0_SHADOW, msr.hi, msr.lo);
>    printk(BIOS_DEBUG, "MSR 0x%08X is now 0x%08X:0x%08X\n",
>           MSR_GLIU1_SHADOW, msr.hi, msr.lo);
>    /* TODO: Is the respective wrmsr() missing? */
> #endif
> }

./device/device.c
> void dev_phase4(void)
> {
>    struct resource *io, *mem;
>    struct device *root;
>
> printk(BIOS_INFO, "Phase 4: Allocating resources...\n");
>
> root = &dev_root;
>    if (!root->ops) {
>    	printk(BIOS_ERR,
>    	       "Phase 4: dev_root missing ops initialization\nPhase 4: Failed.\n");
>    	return;
>    }
>    if (!root->ops->phase4_read_resources) {
>    	printk(BIOS_ERR,
>    	       "dev_root ops missing read_resources\nPhase 4: Failed.\n");
>    	return;
>    }
>
> if (!root->ops->phase4_set_resources) {
>    	printk(BIOS_ERR,
>    	       "dev_root ops missing set_resources\nPhase 4: Failed.\n");
>    	return;
>    }
>
> printk(BIOS_INFO, "Phase 4: Reading resources...\n");
>    root->ops->phase4_read_resources(root);
>    printk(BIOS_INFO, "Phase 4: Done reading resources.\n");
>
> /* Get the resources. */
>    io = &root->resource[0];
>    mem = &root->resource[1];
>
> /* Make certain the I/O devices are allocated somewhere safe. */
>    io->base = DEVICE_IO_START;
>    io->flags |= IORESOURCE_ASSIGNED;
>    io->flags &= ~IORESOURCE_STORED;
>
> /* Now reallocate the PCI resources memory with the
>     * highest addresses I can manage.
>     */
>    mem->base = resource_max(&root->resource[1]);
>    mem->flags |= IORESOURCE_ASSIGNED;
>    mem->flags &= ~IORESOURCE_STORED;
>
> #if defined(CONFIG_PCI_OPTION_ROM_RUN) && CONFIG_PCI_OPTION_ROM_RUN == 1
>    /* Allocate the VGA I/O resource. */
>    allocate_vga_resource();
> #endif
>
> /* Store the computed resource allocations into device registers. */
>    printk(BIOS_INFO, "Phase 4: Setting resources...\n");
>    root->ops->phase4_set_resources(root);
>    printk(BIOS_INFO, "Phase 4: Done setting resources.\n");
> #if 0

Blindly copied from v2 in v3:r69. Was never enabled in v2 (committed
as dead code by Eric Biederman in v2:r1664 with log message
"Updates for 64bit resource support, handling missing devices and cpus
 in the config file").
Kill?
> mem->flags |= IORESOURCE_STORED;
>    report_resource_stored(root, mem, "");
> #endif
>
> printk(BIOS_INFO, "Phase 4: Done allocating resources.\n");
> }

./device/pci_device.c
> struct resource *pci_get_resource(struct device *dev, unsigned long
> index) { struct resource *resource; unsigned long value, attr;
> resource_t moving, limit; /* Initialize the resources to nothing. */
> resource = new_resource(dev, index); /* Get the initial value. */
> value = pci_read_config32(dev, index); /* See which bits move. */
> moving = pci_moving_config32(dev, index); /* Initialize attr to the
> bits that do not move. */ attr = value & ~moving; /* If it is a 64bit
> resource look at the high half as well. */ if (((attr &
> PCI_BASE_ADDRESS_SPACE_IO) == 0) && ((attr &
> PCI_BASE_ADDRESS_MEM_LIMIT_MASK) == PCI_BASE_ADDRESS_MEM_LIMIT_64)) {
> /* Find the high bits that move. */ moving |= ((resource_t)
> pci_moving_config32(dev, index + 4)) << 32; } /* Find the resource
> constraints. * Start by finding the bits that move. From there: * -
> Size is the least significant bit of the bits that move. * - Limit is
> all of the bits that move plus all of the lower bits. * See PCI Spec
> 6.2.5.1. */ limit = 0; if (moving) { resource->size = 1;
> resource->align = resource->gran = 0; while (!(moving &
> resource->size)) { resource->size <<= 1; resource->align += 1;
> resource->gran += 1; } resource->limit = limit = moving |
> (resource->size - 1); } /* Some broken hardware has read-only
> registers that do not * really size correctly. * Example: the Acer
> M7229 has BARs 1-4 normally read-only. * so BAR1 at offset 0x10 reads
> 0x1f1. If you size that register * by writing 0xffffffff to it, it
> will read back as 0x1f1 -- a * violation of the spec. * We catch this
> case and ignore it by observing which bits move, * This also catches
> the common case unimplemented registers * that always read back as 0.
> */ if (moving == 0) { if (value != 0) { printk(BIOS_DEBUG, "%s
> register %02lx(%08lx), read-only ignoring it\n", dev_path(dev), index,
> value); } resource->flags = 0; } else if (attr &
> PCI_BASE_ADDRESS_SPACE_IO) { /* An I/O mapped base address. */ attr &=
> PCI_BASE_ADDRESS_IO_ATTR_MASK; resource->flags |= IORESOURCE_IO; /* I
> don't want to deal with 32bit I/O resources. */ resource->limit =
> 0xffff; } else { /* A Memory mapped base address. */ attr &=
> PCI_BASE_ADDRESS_MEM_ATTR_MASK; resource->flags |= IORESOURCE_MEM; if
> (attr & PCI_BASE_ADDRESS_MEM_PREFETCH) { resource->flags |=
> IORESOURCE_PREFETCH; } attr &= PCI_BASE_ADDRESS_MEM_LIMIT_MASK; if
> (attr == PCI_BASE_ADDRESS_MEM_LIMIT_32) { /* 32bit limit. */
> resource->limit = 0xffffffffUL; } else if (attr ==
> PCI_BASE_ADDRESS_MEM_LIMIT_1M) { /* 1MB limit. */ resource->limit =
> 0x000fffffUL; } else if (attr == PCI_BASE_ADDRESS_MEM_LIMIT_64) { /*
> 64bit limit. */ resource->limit = 0xffffffffffffffffULL;
> resource->flags |= IORESOURCE_PCI64; } else { /* Invalid value. */
> resource->flags = 0; } } /* Don't let the limit exceed which bits can
> move. */ if (resource->limit > limit) { resource->limit = limit; } #if 0

This just disables some debug printing. Enable it on the most verbose level?
> if (resource->flags) { printk(BIOS_DEBUG, "%s %02x ->", dev_path(dev),
> resource->index); printk(BIOS_DEBUG, " value: 0x%08llx zeroes:
> 0x%08llx ones: 0x%08llx attr: %08lx\n", value, zeroes, ones, attr);
> printk(BIOS_DEBUG, "%s %02x -> size: 0x%08llx max: 0x%08llx %s\n ",
> dev_path(dev), resource->index, resource->size, resource->limit,
> resource_type(resource)); } #endif return resource; }
> void pci_level_irq(unsigned char intNum) { unsigned short intBits =
> inb(0x4d0) | (((unsigned)inb(0x4d1)) << 8); printk(BIOS_SPEW, "%s:
> current ints are 0x%x\n", __func__, intBits); intBits |= (1 <<
> intNum); printk(BIOS_SPEW, "%s: try to set ints 0x%x\n", __func__,
> intBits); /* Write new values. */ outb((unsigned char)intBits, 0x4d0);
> outb((unsigned char)(intBits >> 8), 0x4d1); /* This seems like an
> error but is not. */ #if 1

Just remove the #if 1?
> if (inb(0x4d0) != (intBits & 0xff)) { printk(BIOS_ERR, "%s: lower
> order bits are wrong: want 0x%x, got 0x%x\n", __func__, intBits &
> 0xff, inb(0x4d0)); } if (inb(0x4d1) != ((intBits >> 8) & 0xff)) {
> printk(BIOS_ERR, "%s: lower order bits are wrong: want 0x%x, got
> 0x%x\n", __func__, (intBits >> 8) & 0xff, inb(0x4d1)); } #endif }

./arch/x86/serial.c
> void uart_init(void) { unsigned ttysx_div; #if 0

Config option?
> unsigned ttysx_index; static const unsigned char divisor[] = { 1, 2,
> 3, 6, 12, 24, 48, 96 }; // read CMOS settings? ttysx_index =
> read_option(CMOS_VSTART_baud_rate, CMOS_VLEN_baud_rate, 0);
> ttysx_index &= 7; ttysx_div = divisor[ttysx_index]; #else ttysx_div =
> TTYSx_DIV; #endif uart8250_init(TTYSx_BASE, ttysx_div, UART_LCS); }

./arch/x86/archtables.c
Now that file is king of #if 0.
> #warning enable disabled code in archtables.c #if 0 // Copy GDT to new
> location and reload it // 2003-07 by SONE Takeshi // Ported from
> Etherboot to coreboot 2005-08 by Steve Magnani void move_gdt(unsigned
> long newgdt) { u16 num_gdt_bytes = &gdt_end - &gdt; struct gdtarg
> gdtarg; printk(BIOS_DEBUG,"Moving GDT to %#lx...", newgdt);
> memcpy((void*)newgdt, &gdt, num_gdt_bytes); gdtarg.base = newgdt;
> gdtarg.limit = num_gdt_bytes - 1; __asm__ __volatile__ ("lgdt %0\n\t"
> : : "m" (gdtarg)); printk(BIOS_DEBUG,"OK\n"); } #endif struct
> lb_memory *arch_write_tables(void) { #if 0 #if HAVE_MP_TABLE==1
> unsigned long new_low_table_end; #endif #endif unsigned long
> low_table_start, low_table_end; unsigned long rom_table_start,
> rom_table_end; rom_table_start = 0xf0000; rom_table_end = 0xf0000; /*
> Start low addr at 16 bytes instead of 0 because of a buglet * in the
> generic linux unzip code, as it tests for the a20 line. */
> low_table_start = 0; low_table_end = 16;
> post_code(POST_STAGE2_ARCH_WRITE_TABLES_ENTER); /* This table must be
> betweeen 0xf0000 & 0x100000 */ /* we need to make a decision: create
> empty functions * in .h files if the cpp variable is undefined, or
> #ifdef? */ #ifdef CONFIG_PIRQ_TABLE rom_table_end =
> write_pirq_routing_table(rom_table_end); rom_table_end =
> (rom_table_end + 1023) & ~1023; #endif /* Write ACPI tables */ /*
> write them in the rom area because DSDT can be large (8K on epia-m)
> which * pushes coreboot table out of first 4K if set up in low table
> area */ // rom_table_end = write_acpi_tables(rom_table_end); //
> rom_table_end = (rom_table_end+1023) & ~1023; /* copy the smp block to
> address 0 */ post_code(POST_STAGE2_ARCH_WRITE_TABLES_MIDDLE); /* The
> smp table must be in 0-1K, 639K-640K, or 960K-1M */ //
> new_low_table_end = write_smp_table(low_table_end); #if 0 #if
> HAVE_MP_TABLE==1 /* Don't write anything in the traditional x86 BIOS
> data segment, * for example the linux kernel smp need to use 0x467 to
> pass reset vector */ if(new_low_table_end>0x467){ unsigned
> mptable_size = new_low_table_end - low_table_end -
> SMP_FLOATING_TABLE_LEN; /* We can not put mptable here, we need to
> copy them to somewhere else*/
> if((rom_table_end+mptable_size)<0x100000) { /* We can copy mptable on
> rom_table, and leave low space for lbtable */ printk(BIOS_DEBUG,"Move
> mptable to 0x%0x\n", rom_table_end); memcpy((unsigned char
> *)rom_table_end, (unsigned char
> *)(low_table_end+SMP_FLOATING_TABLE_LEN), mptable_size);
> memset((unsigned char *)low_table_end, '\0', mptable_size +
> SMP_FLOATING_TABLE_LEN);
> smp_write_floating_table_physaddr(low_table_end, rom_table_end);
> low_table_end += SMP_FLOATING_TABLE_LEN; rom_table_end +=
> mptable_size; rom_table_end = (rom_table_end+1023) & ~1023; } else {
> /* We can need to put mptable low and from 0x500 */
> printk(BIOS_DEBUG,"Move mptable to 0x%0x\n", 0x500); memcpy((unsigned
> char *)0x500, (unsigned char *)(low_table_end+SMP_FLOATING_TABLE_LEN),
> mptable_size); memset((unsigned char *)low_table_end, '\0',
> 0x500-low_table_end); smp_write_floating_table_physaddr(low_table_end,
> 0x500); low_table_end = 0x500 + mptable_size; } } #endif #endif /*
> Don't write anything in the traditional x86 BIOS data segment */ if
> (low_table_end < 0x500) { low_table_end = 0x500; } #warning GDT should
> be placed in a reserved position from the beginning on. #if 0 //
> Relocate the GDT to reserved memory, so it won't get clobbered
> move_gdt(low_table_end); low_table_end += &gdt_end - &gdt; #endif /*
> The coreboot table must be in 0-4K or 960K-1M */ write_coreboot_table(
> low_table_start, low_table_end, rom_table_start, rom_table_end);
> return get_lb_mem(); }

Sorry, but I couldn't stomach the code above.

./arch/x86/mc146818rtc.c
> void rtc_init(int invalid) { #if defined(CONFIG_OPTION_TABLE) &&
> (CONFIG_OPTION_TABLE == 1) unsigned char x; int cmos_invalid,
> checksum_invalid; #endif printk(BIOS_DEBUG, "Initializing realtime
> clock.\n"); #if defined(CONFIG_OPTION_TABLE) && (CONFIG_OPTION_TABLE
> == 1) /* See if there has been a CMOS power problem. */ x =
> CMOS_READ(RTC_VALID); cmos_invalid = !(x & RTC_VRT); /* See if there
> is a CMOS checksum error */ checksum_invalid =
> !rtc_checksum_valid(PC_CKS_RANGE_START, PC_CKS_RANGE_END,PC_CKS_LOC);
> if (invalid || cmos_invalid || checksum_invalid) {
> printk(BIOS_WARNING, "RTC:%s%s%s zeroing cmos\n", invalid?" Clear
> requested":"", cmos_invalid?" Power Problem":"", checksum_invalid?"
> Checksum invalid":""); #if 0

Is this dependent on whether we want to use NVRAM?
> CMOS_WRITE(0, 0x01); CMOS_WRITE(0, 0x03); CMOS_WRITE(0, 0x05); for(i =
> 10; i < 48; i++) { CMOS_WRITE(0, i); } if (cmos_invalid) { /* Now
> setup a default date of Sat 1 January 2000 */ CMOS_WRITE(0, 0x00); /*
> seconds */ CMOS_WRITE(0, 0x02); /* minutes */ CMOS_WRITE(1, 0x04); /*
> hours */ CMOS_WRITE(7, 0x06); /* day of week */ CMOS_WRITE(1, 0x07);
> /* day of month */ CMOS_WRITE(1, 0x08); /* month */ CMOS_WRITE(0,
> 0x09); /* year */ } #endif } #endif /* Setup the real time clock */
> CMOS_WRITE(RTC_CONTROL_DEFAULT, RTC_CONTROL); /* Setup the frequency
> it operates at */ CMOS_WRITE(RTC_FREQ_SELECT_DEFAULT,
> RTC_FREQ_SELECT); #if defined(CONFIG_OPTION_TABLE) &&
> (CONFIG_OPTION_TABLE == 1) /* See if there is a coreboot CMOS checksum
> error */ checksum_invalid = !rtc_checksum_valid(CB_CKS_RANGE_START,
> CB_CKS_RANGE_END,CB_CKS_LOC); if(checksum_invalid)
> printk(BIOS_WARNING, "Invalid coreboot CMOS checksum.\n"); /* Make
> certain we have a valid checksum */
> rtc_set_checksum(PC_CKS_RANGE_START, PC_CKS_RANGE_END,PC_CKS_LOC);
> #endif /* Clear any pending interrupts */ (void)
> CMOS_READ(RTC_INTR_FLAGS); }

Regards,
Carl-Daniel

[Marc Jones]

Carl-Daniel Hailfinger wrote:
> Hi,
>
> Ron's latest patch triggered me to look at how much dead code we already
> have in v3. The situation is not bad, but we could use more commits
> which add comments before #if 0 and #if 1, just to make sure the code is
> not left in forever without known purpose.
>
> Analysis of the current state follows:
>
> ./southbridge/amd/cs5536/cs5536.c
> > void chipsetinit(void)
> > {
> >    struct device *dev;
> >    struct msr msr;
> >    struct southbridge_amd_cs5536_dts_config *sb;
> >    const struct msrinit *csi;
> >
> > post_code(P80_CHIPSET_INIT);
> >    dev = dev_find_pci_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_dts_config *)dev->device_configuration;
> >
> > #if 0
>
> How about
> #ifdef CONFIG_SUSPEND_TO_RAM

Sounds good.
> > if (!IsS3Resume())
> >    {
> >    	struct acpi_init *aci = acpi_init_table;
> >    	for (/* Nothing */; aci->ioreg; aci++) {
> >    		outl(aci->regdata, aci->ioreg);
> >    		inl(aci->ioreg);
> >    	}
> >    	pm_chipset_init();
> >    }
> > #endif
> >
> > /* Set HD IRQ. */
> >    outl(GPIOL_2_SET, GPIO_IO_BASE + GPIOL_INPUT_ENABLE);
> >    outl(GPIOL_2_SET, GPIO_IO_BASE + GPIOL_IN_AUX1_SELECT);
> > static void southbridge_init(struct device *dev)
> > {
> >    struct southbridge_amd_cs5536_dts_config *sb =
> >        (struct southbridge_amd_cs5536_dts_config *)dev->device_configuration;
> >
> > /*
> >     * struct device *gpiodev;
> >     * unsigned short gpiobase = MDD_GPIO;
> >     */
> >
> > printk(BIOS_ERR, "cs5536: %s\n", __FUNCTION__);
> >
> > setup_i8259();
> >    lpc_init(sb);
> >    uarts_init(sb);
> >
> > if (sb->enable_gpio_int_route) {
> >    	vr_write((VRC_MISCELLANEOUS << 8) + PCI_INT_AB,
> >    		 (sb->enable_gpio_int_route & 0xFFFF));
> >    	vr_write((VRC_MISCELLANEOUS << 8) + PCI_INT_CD,
> >    		 (sb->enable_gpio_int_route >> 16));
> >    }
> >
> > printk(BIOS_ERR, "cs5536: %s: enable_ide_nand_flash is %d\n",
> >           __FUNCTION__, sb->enable_ide_nand_flash);
> >    if (sb->enable_ide_nand_flash == 1)
> >    	enable_ide_nand_flash_header();
> >
> > enable_USB_port4(sb);
> >
> > if (sb->enable_ide)
> >    	ide_init(dev);
> >
> > #warning Add back in unwanted VPCI support
> > #if 0
>
> Any reason not to remove this #if 0?

I think that this #if 0 should be removed and the code included. 
Disabling VPCI headers may be important to some mainboards.
> > /* Disable unwanted virtual PCI devices. */
> >    for (i = 0; (i < MAX_UNWANTED_VPCI) && (0 != sb->unwanted_vpci[i]); i++) {
> >    	printk(BIOS_DEBUG, "Disabling VPCI device: 0x%08X\n",
> >    	       sb->unwanted_vpci[i]);
> >    	outl(sb->unwanted_vpci[i] + 0x7C, 0xCF8);
> >    	outl(0xDEADBEEF, 0xCFC);
> >    }
> > #endif
> >    printk(BIOS_SPEW, "cs5536: %s() Exit\n", __FUNCTION__);
> > }
> >
> > /**
>
> ./northbridge/amd/geodelx/raminit.c
> > static void check_ddr_max(u8 dimm0, u8 dimm1)
> > {
> >    u8 spd_byte0, spd_byte1;
> >    u16 speed;
> >
> > /* PC133 identifier */
> >    spd_byte0 = spd_read_byte(dimm0, SPD_MIN_CYCLE_TIME_AT_CAS_MAX);
> >    if (spd_byte0 == 0xFF)
> >    	spd_byte0 = 0;
> >    spd_byte1 = spd_read_byte(dimm1, SPD_MIN_CYCLE_TIME_AT_CAS_MAX);
> >    if (spd_byte1 == 0xFF)
> >    	spd_byte1 = 0;
> >
> > /* I don't think you need this check. */
> > #if 0
>
> We may need an opinion from someone familiar with the hardware details...

This code can be removed.
> > if (spd_byte0 < 0xA0 || spd_byte0 < 0xA0) {
> >    	printk(BIOS_EMERG, "DIMM overclocked. Check GeodeLink speed\n");
> >    	post_code(POST_PLL_MEM_FAIL);
> >    	hlt();
> >    }
> > #endif
> >
> > /* Use the slowest DIMM. */
> >    if (spd_byte0 < spd_byte1)
> >    	spd_byte0 = spd_byte1;
> >
> > /* Turn SPD ns time into MHz. Check what the asm does to this math. */
> >    speed = 2 * ((10000 / (((spd_byte0 >> 4) * 10) + (spd_byte0 & 0x0F))));
> >
> > /* Current speed > max speed? */
> >    if (geode_link_speed() > speed) {
> >    	printk(BIOS_EMERG, "DIMM overclocked. Check GeodeLink speed\n");
> >    	post_code(POST_PLL_MEM_FAIL);
> >    	hlt();
> >    }
> > }
> > void sdram_set_registers(void)
> > {
> >    struct msr msr;
> >
> > /* Set Timing Control */
> >    msr = rdmsr(MC_CF1017_DATA);
> >    msr.lo &= ~(7 << CF1017_LOWER_RD_TMG_CTL_SHIFT);
> >    if (geode_link_speed() < 334)
> >    	msr.lo |= (3 << CF1017_LOWER_RD_TMG_CTL_SHIFT);
> >    else
> >    	msr.lo |= (4 << CF1017_LOWER_RD_TMG_CTL_SHIFT);
> >    wrmsr(MC_CF1017_DATA, msr);
> >
> > /* Set Refresh Staggering */
> >    msr = rdmsr(MC_CF07_DATA);
> >    msr.lo &= ~0xF0;
> >    msr.lo |= 0x40;		/* Set refresh to 4 SDRAM clocks. */
> >    wrmsr(MC_CF07_DATA, msr);
> >
> > /* Memory Interleave: Set HOI here otherwise default is LOI. */
> > #if 0
>
> Switch to config option or NVRAM option or kill the code altogether?

I think this code should be removed. I haven't ever seen anyone use HOI.
> > msr = rdmsr(MC_CF8F_DATA);
> >    msr.hi |= CF8F_UPPER_HOI_LOI_SET;
> >    wrmsr(MC_CF8F_DATA, msr);
> > #endif
> > }
>
> ./northbridge/amd/geodelx/geodelx.c
> > static void geodelx_northbridge_init(struct device *dev)
> > {
> >    /* struct msr msr; */
> >
> > printk(BIOS_SPEW, ">> Entering northbridge.c: %s\n", __FUNCTION__);
> >
> > enable_shadow(dev);
> >
> > #if 0
>
> No idea about this one.

Remove the code. The GLIU registers are setup elsewhere.
> > /* Swiss cheese */
> >    msr = rdmsr(MSR_GLIU0_SHADOW);
> >
> > msr.hi |= 0x3;
> >    msr.lo |= 0x30000;
> >
> > printk(BIOS_DEBUG, "MSR 0x%08X is now 0x%08X:0x%08X\n",
> >           MSR_GLIU0_SHADOW, msr.hi, msr.lo);
> >    printk(BIOS_DEBUG, "MSR 0x%08X is now 0x%08X:0x%08X\n",
> >           MSR_GLIU1_SHADOW, msr.hi, msr.lo);
> >    /* TODO: Is the respective wrmsr() missing? */
> > #endif
> > }

Marc

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

[Carl-Daniel Hailfinger]

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

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;
Index: LinuxBIOSv3-if0crud/device/Kconfig
===================================================================
--- LinuxBIOSv3-if0crud/device/Kconfig	(Revision 587)
+++ LinuxBIOSv3-if0crud/device/Kconfig	(Arbeitskopie)
@@ -87,3 +87,13 @@

endmenu

+menu "Power management"
+
+config SUSPEND_TO_RAM
+	bool "Suspend-to-RAM (S3)"
+	depends EXPERIMENTAL && BROKEN
+	help
+	  Enable support for Suspend-to-RAM (S3) functionality.
+
+endmenu
+

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

[Carl-Daniel Hailfinger]

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
> 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 
> Acked-by: Stefan Reinauer 
>  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

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

[Carl-Daniel Hailfinger]

Can someone please test this on Geode? Thanks.

On 13.02.2008 01:02, Carl-Daniel Hailfinger wrote:
> 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
> > 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 
> > Acked-by: Stefan Reinauer 
> >  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.

Enable Suspend-to-RAM code based on config option. Revert semantics to
those we had in v2 and before r385.
This causes pm_chipset_init() to be called on Geode LX.

Signed-off-by: Carl-Daniel Hailfinger

Index: LinuxBIOSv3-s3/southbridge/amd/cs5536/cs5536.c
===================================================================
--- LinuxBIOSv3-s3/southbridge/amd/cs5536/cs5536.c	(Revision 590)
+++ LinuxBIOSv3-s3/southbridge/amd/cs5536/cs5536.c	(Arbeitskopie)
@@ -488,8 +488,9 @@
    }
    sb = (struct southbridge_amd_cs5536_dts_config *)dev->device_configuration;

-#if 0
+#ifdef CONFIG_SUSPEND_TO_RAM
    if (!IsS3Resume())
+#endif
    {
    	struct acpi_init *aci = acpi_init_table;
    	for (; aci->ioreg; aci++) {
@@ -498,7 +499,6 @@
    	}
    	pm_chipset_init();
    }
-#endif

/* Set HD IRQ. */
    outl(GPIOL_2_SET, GPIO_IO_BASE + GPIOL_INPUT_ENABLE);
Index: LinuxBIOSv3-s3/device/Kconfig
===================================================================
--- LinuxBIOSv3-s3/device/Kconfig	(Revision 590)
+++ LinuxBIOSv3-s3/device/Kconfig	(Arbeitskopie)
@@ -87,3 +87,13 @@

endmenu

+menu "Power management"
+
+config SUSPEND_TO_RAM
+	bool "Suspend-to-RAM (S3)"
+	depends EXPERIMENTAL && BROKEN
+	help
+	  Enable support for Suspend-to-RAM (S3) functionality.
+
+endmenu
+

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

[Marc Jones]

Carl-Daniel Hailfinger wrote:
> Can someone please test this on Geode? Thanks.

Builds and run to filo.
Acked-by: Marc Jones

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

[Carl-Daniel Hailfinger]

On 13.02.2008 22:21, Marc Jones wrote:
> Carl-Daniel Hailfinger wrote:
> > Can someone please test this on Geode? Thanks.
>
> Builds and run to filo.
> Acked-by: Marc Jones

Thanks, r595.

Regards,
Carl-Daniel

[Carl-Daniel Hailfinger]

Remove some remaining code inside #if 0.

Signed-off-by: Carl-Daniel Hailfinger

Index: LinuxBIOSv3-if0crud/northbridge/amd/geodelx/raminit.c
===================================================================
--- LinuxBIOSv3-if0crud/northbridge/amd/geodelx/raminit.c	(Revision 591)
+++ LinuxBIOSv3-if0crud/northbridge/amd/geodelx/raminit.c	(Arbeitskopie)
@@ -216,15 +216,6 @@
    if (spd_byte1 == 0xFF)
    	spd_byte1 = 0;

-	/* I don't think you need this check. */
-#if 0
-	if (spd_byte0 < 0xA0 || spd_byte0 < 0xA0) {
-		printk(BIOS_EMERG, "DIMM overclocked. Check GeodeLink speed\n");
-		post_code(POST_PLL_MEM_FAIL);
-		hlt();
-	}
-#endif
-
    /* Use the slowest DIMM. */
    if (spd_byte0 < spd_byte1)
    	spd_byte0 = spd_byte1;
@@ -580,13 +571,6 @@
    msr.lo &= ~0xF0;
    msr.lo |= 0x40;		/* Set refresh to 4 SDRAM clocks. */
    wrmsr(MC_CF07_DATA, msr);
-
-	/* Memory Interleave: Set HOI here otherwise default is LOI. */
-#if 0
-	msr = rdmsr(MC_CF8F_DATA);
-	msr.hi |= CF8F_UPPER_HOI_LOI_SET;
-	wrmsr(MC_CF8F_DATA, msr);
-#endif
 }

/**
Index: LinuxBIOSv3-if0crud/northbridge/amd/geodelx/geodelx.c
===================================================================
--- LinuxBIOSv3-if0crud/northbridge/amd/geodelx/geodelx.c	(Revision 591)
+++ LinuxBIOSv3-if0crud/northbridge/amd/geodelx/geodelx.c	(Arbeitskopie)
@@ -163,20 +163,6 @@
    printk(BIOS_SPEW, ">> Entering northbridge.c: %s\n", __FUNCTION__);

enable_shadow(dev);
-
-#if 0
-	/* Swiss cheese */
-	msr = rdmsr(MSR_GLIU0_SHADOW);
-
-	msr.hi |= 0x3;
-	msr.lo |= 0x30000;
-
-	printk(BIOS_DEBUG, "MSR 0x%08X is now 0x%08X:0x%08X\n",
-	       MSR_GLIU0_SHADOW, msr.hi, msr.lo);
-	printk(BIOS_DEBUG, "MSR 0x%08X is now 0x%08X:0x%08X\n",
-	       MSR_GLIU1_SHADOW, msr.hi, msr.lo);
-	/* TODO: Is the respective wrmsr() missing? */
-#endif
 }

/**

[ron minnich]

we definitely need the ack to be from Marc :-)

ron

[Marc Jones]

Carl-Daniel Hailfinger wrote:
> Remove some remaining code inside #if 0.
>
> Signed-off-by: Carl-Daniel Hailfinger

Builds and run to filo.
Acked-by: Marc Jones

[Carl-Daniel Hailfinger]

On 13.02.2008 22:21, Marc Jones wrote:
> Carl-Daniel Hailfinger wrote:
> > Remove some remaining code inside #if 0.
> >
> > Signed-off-by: Carl-Daniel Hailfinger
>
> Builds and run to filo.
> Acked-by: Marc Jones

Thanks, r596.

Regards,
Carl-Daniel

[PATCH] v3: Kill dead code

[Carl-Daniel Hailfinger]

Kill dead code inside #if 0.

The code was blindly copied from v2 in v3:r69. Was never enabled in v2
(committed as dead code by Eric Biederman in v2:r1664 with log message
"Updates for 64bit resource support, handling missing devices and cpus
in the config file").

Signed-off-by: Carl-Daniel Hailfinger

Index: LinuxBIOSv3-if0crud-next/device/pci_device.c
===================================================================
--- LinuxBIOSv3-if0crud-next/device/pci_device.c	(Revision 596)
+++ LinuxBIOSv3-if0crud-next/device/pci_device.c	(Arbeitskopie)
@@ -247,19 +247,6 @@
    if (resource->limit > limit) {
    	resource->limit = limit;
    }
-#if 0
-	if (resource->flags) {
-		printk(BIOS_DEBUG, "%s %02x ->",
-		       dev_path(dev), resource->index);
-		printk(BIOS_DEBUG,
-		       " value: 0x%08llx zeroes: 0x%08llx ones: 0x%08llx attr: %08lx\n",
-		       value, zeroes, ones, attr);
-		printk(BIOS_DEBUG,
-		       "%s %02x -> size: 0x%08llx max: 0x%08llx %s\n ",
-		       dev_path(dev), resource->index, resource->size,
-		       resource->limit, resource_type(resource));
-	}
-#endif

return resource;
 }

[PATCH] v3: Kill dead code

[ron minnich]

Acked-by: Ronald G. Minnich

On Wed, Feb 13, 2008 at 3:03 PM, Carl-Daniel Hailfinger
 wrote:
> Kill dead code inside #if 0.
>
> The code was blindly copied from v2 in v3:r69. Was never enabled in v2
>  (committed as dead code by Eric Biederman in v2:r1664 with log message
>  "Updates for 64bit resource support, handling missing devices and cpus
>  in the config file").
>
> Signed-off-by: Carl-Daniel Hailfinger
> Index: LinuxBIOSv3-if0crud-next/device/pci_device.c
>
> --- LinuxBIOSv3-if0crud-next/device/pci_device.c        (Revision 596)
>  +++ LinuxBIOSv3-if0crud-next/device/pci_device.c        (Arbeitskopie)
>  @@ -247,19 +247,6 @@
>         if (resource->limit > limit) {
>                 resource->limit = limit;
>         }
>  -#if 0
>   if (resource->flags) {
>           printk(BIOS_DEBUG, "%s %02x ->",
>                  dev_path(dev), resource->index);
>           printk(BIOS_DEBUG,
>                  " value: 0x%08llx zeroes: 0x%08llx ones: 0x%08llx attr: %08lx\n",
>                  value, zeroes, ones, attr);
>           printk(BIOS_DEBUG,
>                  "%s %02x -> size: 0x%08llx max: 0x%08llx %s\n ",
>                  dev_path(dev), resource->index, resource->size,
>                  resource->limit, resource_type(resource));
>   }
>
> -#endif
>     return resource;
>
> }

[PATCH] v3: Kill dead code

[Carl-Daniel Hailfinger]

On 14.02.2008 17:55, ron minnich wrote:
> Acked-by: Ronald G. Minnich

Thanks, r597.

Regards,
Carl-Daniel

[PATCH] v3: remove superfluous #if 1

[Carl-Daniel Hailfinger]

Remove superfluous #if 1. The code has been enabled since it was
committed, the #if 1 never served any purpose.

Signed-off-by: Carl-Daniel Hailfinger

Index: LinuxBIOSv3-if0crud/device/pci_device.c
===================================================================
--- LinuxBIOSv3-if0crud/device/pci_device.c	(Revision 596)
+++ LinuxBIOSv3-if0crud/device/pci_device.c	(Arbeitskopie)
@@ -1284,7 +1284,6 @@
    outb((unsigned char)(intBits >> 8), 0x4d1);

/* This seems like an error but is not. */
-#if 1
    if (inb(0x4d0) != (intBits & 0xff)) {
    	printk(BIOS_ERR,
    	       "%s: lower order bits are wrong: want 0x%x, got 0x%x\n",
@@ -1295,7 +1294,6 @@
    	       "%s: lower order bits are wrong: want 0x%x, got 0x%x\n",
    	       __func__, (intBits >> 8) & 0xff, inb(0x4d1));
    }
-#endif
 }

/**

[PATCH] v3: remove superfluous #if 1

[Stefan Reinauer]

Carl-Daniel Hailfinger wrote:
> Remove superfluous #if 1. The code has been enabled since it was
> committed, the #if 1 never served any purpose.
>
> Signed-off-by: Carl-Daniel Hailfinger

Acked-by: Stefan Reinauer
> Index: LinuxBIOSv3-if0crud/device/pci_device.c
>
> --- LinuxBIOSv3-if0crud/device/pci_device.c	(Revision 596)
> +++ LinuxBIOSv3-if0crud/device/pci_device.c	(Arbeitskopie)
> @@ -1284,7 +1284,6 @@
>    outb((unsigned char)(intBits >> 8), 0x4d1);
>
> /* This seems like an error but is not. */
> -#if 1
>    if (inb(0x4d0) != (intBits & 0xff)) {
>    	printk(BIOS_ERR,
>    	       "%s: lower order bits are wrong: want 0x%x, got 0x%x\n",
> @@ -1295,7 +1294,6 @@
>    	       "%s: lower order bits are wrong: want 0x%x, got 0x%x\n",
>    	       __func__, (intBits >> 8) & 0xff, inb(0x4d1));
>    }
> -#endif
>  }
>
> /**

[PATCH] v3: remove superfluous #if 1

[Carl-Daniel Hailfinger]

On 14.02.2008 19:45, Stefan Reinauer wrote:
> Carl-Daniel Hailfinger wrote:
> > Remove superfluous #if 1. The code has been enabled since it was
> > committed, the #if 1 never served any purpose.
> >
> > Signed-off-by: Carl-Daniel Hailfinger
>
> Acked-by: Stefan Reinauer

Thanks, r598.

Regards,
Carl-Daniel

[Carl-Daniel Hailfinger]

device/device.c has code which was dead from the beginning when it was
copied from v2 to v3. It was never enabled in v2 as well. Then again, it
looks like enabling the code might make sense. See below.
> void dev_phase4(void)
> {
>    struct resource *io, *mem;
>    struct device *root;
>
> printk(BIOS_INFO, "Phase 4: Allocating resources...\n");
>
> root = &dev_root;
>    if (!root->ops) {
>    	printk(BIOS_ERR,
>    	       "Phase 4: dev_root missing ops initialization\nPhase 4: Failed.\n");
>    	return;
>    }
>    if (!root->ops->phase4_read_resources) {
>    	printk(BIOS_ERR,
>    	       "dev_root ops missing read_resources\nPhase 4: Failed.\n");
>    	return;
>    }
>
> if (!root->ops->phase4_set_resources) {
>    	printk(BIOS_ERR,
>    	       "dev_root ops missing set_resources\nPhase 4: Failed.\n");
>    	return;
>    }
>
> printk(BIOS_INFO, "Phase 4: Reading resources...\n");
>    root->ops->phase4_read_resources(root);
>    printk(BIOS_INFO, "Phase 4: Done reading resources.\n");
>
> /* Get the resources. */
>    io = &root->resource[0];
>    mem = &root->resource[1];
>
> /* Make certain the I/O devices are allocated somewhere safe. */
>    io->base = DEVICE_IO_START;
>    io->flags |= IORESOURCE_ASSIGNED;
>    io->flags &= ~IORESOURCE_STORED;
>
> /* Now reallocate the PCI resources memory with the
>     * highest addresses I can manage.
>     */
>    mem->base = resource_max(&root->resource[1]);
>    mem->flags |= IORESOURCE_ASSIGNED;
>    mem->flags &= ~IORESOURCE_STORED;
>
> #if defined(CONFIG_PCI_OPTION_ROM_RUN) && CONFIG_PCI_OPTION_ROM_RUN == 1
>    /* Allocate the VGA I/O resource. */
>    allocate_vga_resource();
> #endif
>
> /* Store the computed resource allocations into device registers. */
>    printk(BIOS_INFO, "Phase 4: Setting resources...\n");
>    root->ops->phase4_set_resources(root);
>    printk(BIOS_INFO, "Phase 4: Done setting resources.\n");
> #if 0

Here. The resource allocations have been stored into the device in
root->ops->phase4_set_resources(root); so we probably should set the
IORESOURCE_STORED flag. Then again, that should probably be done inside
phase4_set_resources().
Opinions?
> mem->flags |= IORESOURCE_STORED;
>    report_resource_stored(root, mem, "");
> #endif
>
> printk(BIOS_INFO, "Phase 4: Done allocating resources.\n");
> }

Regards,
Carl-Daniel