On Sun, Feb 21, 2010 at 08:11:51AM +0100, Stefan Reinauer wrote:
> This is a general cleanup patch
> - drop include/part and move files to include/
> - get rid lots of warnings 
> - make resource allocator happy with w83627thg
> - trivial cbmem resume fix
> - fix payload and log level settings in abuild
> - fix kontron mptable for virtual wire mode
> - drop some dead includes and dead code. 
> 
> Signed-off-by: Stefan Reinauer <ste...@coresystems.de>
 
Nice!

  
> --- src/southbridge/intel/i82801gx/i82801gx_pcie.c    (revision 5133)
> +++ src/southbridge/intel/i82801gx/i82801gx_pcie.c    (working copy)
> @@ -75,7 +75,7 @@
>       reg16 |= (1 << 6);
>       pci_write_config16(dev, 0x50, reg16);
>  
> -#if EVEN_MORE_DEBUG
> +#ifdef EVEN_MORE_DEBUG

Maybe this should be merged into the general prink infrastructure
as SPEW messages etc?


> --- src/devices/pnp_device.c  (revision 5133)
> +++ src/devices/pnp_device.c  (working copy)
> @@ -240,7 +240,25 @@
>               resource = new_resource(dev, PNP_IDX_DRQ1);
>               resource->size = 1;
>               resource->flags |= IORESOURCE_DRQ;
> -     }       
> +     }
> +     /* These are not IRQs, but set the flag to have the
> +      * resource allocator do the right thing
> +      */

Can you elaborate here? What does "do the right thing" do exactly?


> +     if (info->flags & PNP_EN) {
> +             resource = new_resource(dev, PNP_IDX_EN);
> +             resource->size = 1;
> +             resource->flags |= IORESOURCE_IRQ;
> +     }
> +     if (info->flags & PNP_MSC0) {
> +             resource = new_resource(dev, PNP_IDX_MSC0);
> +             resource->size = 1;
> +             resource->flags |= IORESOURCE_IRQ;
> +     }
> +     if (info->flags & PNP_MSC1) {
> +             resource = new_resource(dev, PNP_IDX_MSC1);
> +             resource->size = 1;
> +             resource->flags |= IORESOURCE_IRQ;
> +     }
>  } 
  

> --- src/include/fallback.h    (revision 0)
> +++ src/include/fallback.h    (revision 0)
> @@ -0,0 +1,18 @@
> +#ifndef FALLBACK_H
> +#define FALLBACK_H

Please add the usual license header while we're at it.


> --- src/include/watchdog.h    (revision 0)
> +++ src/include/watchdog.h    (revision 0)
> @@ -0,0 +1,10 @@
> +#ifndef WATCHDOG_H
> +#define WATCHDOG_H

Ditto.


> --- src/include/reset.h       (revision 0)
> +++ src/include/reset.h       (revision 0)
> @@ -0,0 +1,11 @@
> +#ifndef RESET_H
> +#define RESET_H

Ditto.


> --- src/lib/cbmem.c   (revision 5133)
> +++ src/lib/cbmem.c   (working copy)
> @@ -184,7 +184,7 @@
>       if (acpi_slp_type == 3) {
>               if (!cbmem_reinit(high_tables_base)) {
>                       /* Something went wrong, our high memory area got wiped 
> */
> -                     acpi_slp_type == 0;
> +                     acpi_slp_type = 0;

Nice catch!


> --- src/cpu/intel/model_6ex/cache_as_ram_disable.c    (revision 5133)
> +++ src/cpu/intel/model_6ex/cache_as_ram_disable.c    (working copy)
> @@ -54,8 +54,7 @@
>       real_main(bist);
>  
>       /* No servicable parts below this line .. */
> -
> -#if CAR_DEBUG
> +#ifdef CAR_DEBUG

This should also use the usualy printk infrastructure, or (if it should
remain independent of the loglevel) become a kconfig option in the Debug menu.

  
> -     /* Firewire 4:0.0 */
> +     /* Internal PCI bus (Firewire, PCI slot) */
>       if (firewire) {
> -             smp_write_intsrc(mc, mp_INT, 
> MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, firewire_bus, 0x0, 0x2, 0x10);
> +             smp_write_intsrc(mc, mp_INT, 
> MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, firewire_bus, 0x0, ioapic_id, 0x10);
> +             smp_write_intsrc(mc, mp_INT, 
> MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, firewire_bus, 0x4, ioapic_id, 0x14);
>       }
>  
>       if (riser) {
>               /* Old riser card */
>               // riser slot top 5:8.0
> -             smp_write_intsrc(mc, mp_INT, 
> MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, riser_bus, 0x20, 0x2, 0x14);
> +             smp_write_intsrc(mc, mp_INT, 
> MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, riser_bus, 0x20, ioapic_id, 0x14);
>               // riser slot middle 5:9.0
> -             smp_write_intsrc(mc, mp_INT, 
> MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, riser_bus, 0x24, 0x2, 0x15);
> +             smp_write_intsrc(mc, mp_INT, 
> MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, riser_bus, 0x24, ioapic_id, 0x15);
>               // riser slot bottom 5:a.0
> -             smp_write_intsrc(mc, mp_INT, 
> MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, riser_bus, 0x28, 0x2, 0x16);
> +             smp_write_intsrc(mc, mp_INT, 
> MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, riser_bus, 0x28, ioapic_id, 0x16);
>  
>               /* New Riser Card */
> -             smp_write_intsrc(mc, mp_INT, 
> MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, riser_bus, 0x30, 0x2, 0x14);
> -             smp_write_intsrc(mc, mp_INT, 
> MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, riser_bus, 0x34, 0x2, 0x15);
> -             smp_write_intsrc(mc, mp_INT, 
> MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, riser_bus, 0x38, 0x2, 0x16);
> +             smp_write_intsrc(mc, mp_INT, 
> MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, riser_bus, 0x30, ioapic_id, 0x14);
> +             smp_write_intsrc(mc, mp_INT, 
> MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, riser_bus, 0x34, ioapic_id, 0x15);
> +             smp_write_intsrc(mc, mp_INT, 
> MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, riser_bus, 0x38, ioapic_id, 0x16);
>       }
>  
> +     /* PCIe slot */
> +     smp_write_intsrc(mc, mp_INT, MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, 
> 0x1, 0x0, ioapic_id, 0x10);
> +     smp_write_intsrc(mc, mp_INT, MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, 
> 0x1, 0x1, ioapic_id, 0x11);

Some of the above looks like actual improved board support (not purely
cosmetic), can you please update the board wiki page accordingly?


  
> --- src/arch/i386/boot/pirq_routing.c (revision 5133)
> +++ src/arch/i386/boot/pirq_routing.c (working copy)
> @@ -3,7 +3,11 @@
>  #include <string.h>
>  #include <device/pci.h>
>  
> -#if (CONFIG_DEBUG==1 && CONFIG_GENERATE_PIRQ_TABLE==1)
> +#ifndef CONFIG_IRQ_SLOT_COUNT
> +#warning "CONFIG_IRQ_SLOT_COUNT is not defined. PIRQ won't work correctly."
> +#endif
[...]  
> -#if (CONFIG_PIRQ_ROUTE==1 && CONFIG_GENERATE_PIRQ_TABLE==1)
> +#if CONFIG_PIRQ_ROUTE

Is this intentional? That eliminates the CONFIG_GENERATE_PIRQ_TABLE check,
which is being used in the code base IIRC.


> --- src/arch/i386/include/arch/coreboot_tables.h      (revision 0)
> +++ src/arch/i386/include/arch/coreboot_tables.h      (revision 0)
> @@ -0,0 +1,25 @@
> +#ifndef COREBOOT_TABLE_H
> +#define COREBOOT_TABLE_H
[...]
> +#endif /* COREBOOT_TABLE_H */

You renamed coreboot_table.h to coreboot_tables.h here, so the define
name should also match.


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.randomprojects.org
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org

-- 
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Reply via email to