v2[PATCH]ICH series halt tco timer

This patch halts the tco timer early in the boot process on all ICH  
series southbridges. It also keeps the boot processes from rebooting  
through out the coreboot process.

Signed-off-by: Joseph Smith

Thanks - Joe

[Corey Osgood]

On Sat, Apr 5, 2008 at 8:24 PM,  wrote:
> This patch halts the tco timer early in the boot process on all ICH series
> southbridges. It also keeps the boot processes from rebooting through out
> the coreboot process.
>
> Signed-off-by: Joseph Smith
>
> Thanks - Joe

Index: src/southbridge/intel/i82801xx/i82801xx_early_lpc.c
> ===================================================================
> --- src/southbridge/intel/i82801xx/i82801xx_early_lpc.c    (revision 0)
> +++ src/southbridge/intel/i82801xx/i82801xx_early_lpc.c    (revision 0)
> @@ -0,0 +1,44 @@
> +/*
> This file is part of the coreboot project.
> Copyright (C) 2008 Joseph Smith
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License version 2 as
> published by the Free Software Foundation.
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU General Public License for more details.
> You should have received a copy of the GNU General Public License
> along with this program; if not, write to the Free Software
> Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301
>
> USA
> */
>
> +#define PMBASE_ADDR    0x0400 /* ACPI Base Address Register */
> +#define TCOBASE        0x60 /* TCO Base Address Register */
> +#define TCO1_CNT    0x08 /* TCO1 Control Register */

These should go into i82801xx.h, along with the defines in i82801xx_lpc.h.
Do that, and this patch is
Acked-by: Corey Osgood
> +static void i82801xx_halt_tco_timer(void)
> +{
> device_t dev;
> uint16_t halt_tco_timer;
> /* Set the LPC device statically. */
> dev = PCI_DEV(0x0, 0x1f, 0x0);
> /* Temporarily set ACPI base address (I/O space). */
> pci_write_config32(dev, PMBASE, (PMBASE_ADDR | 1));

+
> /* Temporarily enable ACPI I/O. */
> pci_write_config8(dev, ACPI_CNTL, 0x10);
> /* Halt the TCO timer, preventing SMI and automatic reboot */
> outw(inw(PMBASE_ADDR + TCOBASE + TCO1_CNT) | (1 << 11), PMBASE_ADDR +
>
> TCOBASE + TCO1_CNT);
> /* Disable ACPI I/O. */
> pci_write_config8(dev, ACPI_CNTL, 0x00);
>
> +}
> Index: src/mainboard/rca/rm4100/auto.c
> ===================================================================
> --- src/mainboard/rca/rm4100/auto.c    (revision 3217)
> +++ src/mainboard/rca/rm4100/auto.c    (working copy)
> @@ -42,6 +42,7 @@
>  #define SERIAL_DEV PNP_DEV(0x2e, SMSCSUPERIO_SP1)
>
> #include "southbridge/intel/i82801xx/i82801xx_early_smbus.c"
> +#include "southbridge/intel/i82801xx/i82801xx_early_lpc.c"
>
> /**
> The onboard 128MB PC133 memory does not have a SPD EEPROM so the
>
> @@ -69,24 +70,6 @@
>  #include "sdram/generic_sdram.c"
>
> /**
> We have to disable the TCO Timer system reboot feature
> or we get several reboots through out the boot processes.
> */
>
> -static void disable_tco_timer(void)
> -{
> device_t dev;
> u8 reg8;
> /* Set the LPC device statically. */
> dev = PCI_DEV(0x0, 0x1f, 0x0);
> /* Disable the TCO Timer system reboot feature. */
> reg8 = pci_read_config8(dev, 0xd4);
> reg8 |= (1 << 1);
> pci_write_config8(dev, 0xd4, reg8);
>
> -}
>
> -/**
> The AC'97 Audio Controller I/O space registers are read only by
>
> default
> so we need to enable them by setting register 0x41 to 0x01.
>
> */
> @@ -131,6 +114,6 @@
>      /* ram_check(0, 640 * 1024); */
>      /* ram_check(130048 * 1024, 131072 * 1024); */
> disable_tco_timer();
> i82801xx_halt_tco_timer();
> ac97_io_enable();
>
> }

Quoting Corey Osgood :
> On Sat, Apr 5, 2008 at 8:24 PM,  wrote:
> > This patch halts the tco timer early in the boot process on all ICH series
> > southbridges. It also keeps the boot processes from rebooting through out
> > the coreboot process.
> >
> > Signed-off-by: Joseph Smith
> >
> > Thanks - Joe
>
> Index: src/southbridge/intel/i82801xx/i82801xx_early_lpc.c
> > ===================================================================
> > --- src/southbridge/intel/i82801xx/i82801xx_early_lpc.c    (revision 0)
> > +++ src/southbridge/intel/i82801xx/i82801xx_early_lpc.c    (revision 0)
> > @@ -0,0 +1,44 @@
> > +/*
> > This file is part of the coreboot project.
> > Copyright (C) 2008 Joseph Smith
> > This program is free software; you can redistribute it and/or modify
> > it under the terms of the GNU General Public License version 2 as
> > published by the Free Software Foundation.
> > This program is distributed in the hope that it will be useful,
> > but WITHOUT ANY WARRANTY; without even the implied warranty of
> > MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > GNU General Public License for more details.
> > You should have received a copy of the GNU General Public License
> > along with this program; if not, write to the Free Software
> > Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301
> >
> > USA
> > */
> >
> > +#define PMBASE_ADDR    0x0400 /* ACPI Base Address Register */
> > +#define TCOBASE        0x60 /* TCO Base Address Register */
> > +#define TCO1_CNT    0x08 /* TCO1 Control Register */
>
> These should go into i82801xx.h, along with the defines in i82801xx_lpc.h.
> Do that, and this patch is
> Acked-by: Corey Osgood

I can move the TCOBASE TCO1_CNT to i82801xx.h because these are static but,
PMBASE_ADDR is a variable and should not be placed in a header file correct?
> > +static void i82801xx_halt_tco_timer(void)
> > +{
> > device_t dev;
> > uint16_t halt_tco_timer;
> > /* Set the LPC device statically. */
> > dev = PCI_DEV(0x0, 0x1f, 0x0);
> > /* Temporarily set ACPI base address (I/O space). */
> > pci_write_config32(dev, PMBASE, (PMBASE_ADDR | 1));
> > /* Temporarily enable ACPI I/O. */
> > pci_write_config8(dev, ACPI_CNTL, 0x10);
> > /* Halt the TCO timer, preventing SMI and automatic reboot */
> > outw(inw(PMBASE_ADDR + TCOBASE + TCO1_CNT) | (1 << 11), PMBASE_ADDR +
> >
> > TCOBASE + TCO1_CNT);
> > /* Disable ACPI I/O. */
> > pci_write_config8(dev, ACPI_CNTL, 0x00);
> >
> > +}
> > Index: src/mainboard/rca/rm4100/auto.c
> > ===================================================================
> > --- src/mainboard/rca/rm4100/auto.c    (revision 3217)
> > +++ src/mainboard/rca/rm4100/auto.c    (working copy)
> > @@ -42,6 +42,7 @@
> >  #define SERIAL_DEV PNP_DEV(0x2e, SMSCSUPERIO_SP1)
> >
> > #include "southbridge/intel/i82801xx/i82801xx_early_smbus.c"
> > +#include "southbridge/intel/i82801xx/i82801xx_early_lpc.c"
> >
> > /**
> > The onboard 128MB PC133 memory does not have a SPD EEPROM so the
> >
> > @@ -69,24 +70,6 @@
> >  #include "sdram/generic_sdram.c"
> >
> > /**
> > We have to disable the TCO Timer system reboot feature
> > or we get several reboots through out the boot processes.
> > */
> >
> > -static void disable_tco_timer(void)
> > -{
> > device_t dev;
> > u8 reg8;
> > /* Set the LPC device statically. */
> > dev = PCI_DEV(0x0, 0x1f, 0x0);
> > /* Disable the TCO Timer system reboot feature. */
> > reg8 = pci_read_config8(dev, 0xd4);
> > reg8 |= (1 << 1);
> > pci_write_config8(dev, 0xd4, reg8);
> >
> > -}
> >
> > -/**
> > The AC'97 Audio Controller I/O space registers are read only by
> >
> > default
> > so we need to enable them by setting register 0x41 to 0x01.
> >
> > */
> > @@ -131,6 +114,6 @@
> >      /* ram_check(0, 640 * 1024); */
> >      /* ram_check(130048 * 1024, 131072 * 1024); */
> > disable_tco_timer();
> > i82801xx_halt_tco_timer();
> > ac97_io_enable();
> >
> > }

Thanks - Joe

[Corey Osgood]

On Sat, Apr 5, 2008 at 11:06 PM,  wrote:
> Quoting Corey Osgood :
>
> On Sat, Apr 5, 2008 at 8:24 PM,  wrote:
> > This patch halts the tco timer early in the boot process on all ICH
> > > series
> > > southbridges. It also keeps the boot processes from rebooting through
> > > out
> > > the coreboot process.
> > >
> > > Signed-off-by: Joseph Smith
> > >
> > > Thanks - Joe
> >
> > Index: src/southbridge/intel/i82801xx/i82801xx_early_lpc.c
> > > ===================================================================
> > > --- src/southbridge/intel/i82801xx/i82801xx_early_lpc.c    (revision
> > > 0)
> > > +++ src/southbridge/intel/i82801xx/i82801xx_early_lpc.c    (revision
> > > 0)
> > > @@ -0,0 +1,44 @@
> > > +/*
> > > This file is part of the coreboot project.
> > > Copyright (C) 2008 Joseph Smith
> > > This program is free software; you can redistribute it and/or
> > >
> > > modify
> > > it under the terms of the GNU General Public License version 2 as
> > > published by the Free Software Foundation.
> > > This program is distributed in the hope that it will be useful,
> > > but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > GNU General Public License for more details.
> > > You should have received a copy of the GNU General Public License
> > > along with this program; if not, write to the Free Software
> > > Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> > >
> > > 02110-1301
> > > USA
> > > */
> > >
> > > +#define PMBASE_ADDR    0x0400 /* ACPI Base Address Register */
> > > +#define TCOBASE        0x60 /* TCO Base Address Register */
> > > +#define TCO1_CNT    0x08 /* TCO1 Control Register */
> >
> > These should go into i82801xx.h, along with the defines in
> > i82801xx_lpc.h.
> > Do that, and this patch is
> > Acked-by: Corey Osgood
> >
> > I can move the TCOBASE TCO1_CNT to i82801xx.h because these are static
>
> but,
> PMBASE_ADDR is a variable and should not be placed in a header file
> correct?

PMBASE_ADDR can be anywhere we want it to be, so long as it doesn't collide
with any IO addresses that have been reserved for other things. Currently,
it's defined in i82801xx_lpc.c at the same address, that's why I was saying
to move those defines as well (so PMBASE_ADDR isn't defined in 2 different
places). If PMBASE_ADDR ever needs to be somewhere else for some reason in
the future, we can make it configurable at build time (or something).

-Corey
> > > +static void i82801xx_halt_tco_timer(void)
> > > +{
> > >
> > > device_t dev;
> > >
> > > uint16_t halt_tco_timer;
> > >
> > > /* Set the LPC device statically. */
> > >
> > > dev = PCI_DEV(0x0, 0x1f, 0x0);
> > >
> > > /* Temporarily set ACPI base address (I/O space). */
> > >
> > > pci_write_config32(dev, PMBASE, (PMBASE_ADDR | 1));
> > > /* Temporarily enable ACPI I/O. */
> > > pci_write_config8(dev, ACPI_CNTL, 0x10);
> > > /* Halt the TCO timer, preventing SMI and automatic reboot */
> > > outw(inw(PMBASE_ADDR + TCOBASE + TCO1_CNT) | (1 << 11),
> > >
> > > PMBASE_ADDR +
> > > TCOBASE + TCO1_CNT);
> > > /* Disable ACPI I/O. */
> > > pci_write_config8(dev, ACPI_CNTL, 0x00);
> > >
> > > +}
> > > Index: src/mainboard/rca/rm4100/auto.c
> > > ===================================================================
> > > --- src/mainboard/rca/rm4100/auto.c    (revision 3217)
> > > +++ src/mainboard/rca/rm4100/auto.c    (working copy)
> > > @@ -42,6 +42,7 @@
> > >  #define SERIAL_DEV PNP_DEV(0x2e, SMSCSUPERIO_SP1)
> > >
> > > #include "southbridge/intel/i82801xx/i82801xx_early_smbus.c"
> > > +#include "southbridge/intel/i82801xx/i82801xx_early_lpc.c"
> > >
> > > /**
> > > The onboard 128MB PC133 memory does not have a SPD EEPROM so the
> > >
> > > @@ -69,24 +70,6 @@
> > >  #include "sdram/generic_sdram.c"
> > >
> > > /**
> > > We have to disable the TCO Timer system reboot feature
> > > or we get several reboots through out the boot processes.
> > > */
> > >
> > > -static void disable_tco_timer(void)
> > > -{
> > > device_t dev;
> > > u8 reg8;
> > > /* Set the LPC device statically. */
> > > dev = PCI_DEV(0x0, 0x1f, 0x0);
> > > /* Disable the TCO Timer system reboot feature. */
> > > reg8 = pci_read_config8(dev, 0xd4);
> > > reg8 |= (1 << 1);
> > > pci_write_config8(dev, 0xd4, reg8);
> > >
> > > -}
> > >
> > > -/**
> > > The AC'97 Audio Controller I/O space registers are read only by
> > >
> > > default
> > > so we need to enable them by setting register 0x41 to 0x01.
> > >
> > > */
> > > @@ -131,6 +114,6 @@
> > >     /* ram_check(0, 640 * 1024); */
> > >     /* ram_check(130048 * 1024, 131072 * 1024); */
> > > disable_tco_timer();
> > > i82801xx_halt_tco_timer();
> > > ac97_io_enable();
> > >
> > > }
>
> Thanks - Joe

> PMBASE_ADDR can be anywhere we want it to be, so long as it doesn't collide
> with any IO addresses that have been reserved for other things. Currently,
> it's defined in i82801xx_lpc.c at the same address, that's why I was saying
> to move those defines as well (so PMBASE_ADDR isn't defined in 2 different
> places). If PMBASE_ADDR ever needs to be somewhere else for some reason in
> the future, we can make it configurable at build time (or something).
>
> -Corey

Ok, here is the revised patch.

Signed-off-by: Joseph Smith

Thanks - Joe

[Corey Osgood]

On Sat, Apr 5, 2008 at 11:38 PM,  wrote:
> PMBASE_ADDR can be anywhere we want it to be, so long as it doesn't
> > collide
> > with any IO addresses that have been reserved for other things.
> > Currently,
> > it's defined in i82801xx_lpc.c at the same address, that's why I was
> > saying
> > to move those defines as well (so PMBASE_ADDR isn't defined in 2
> > different
> > places). If PMBASE_ADDR ever needs to be somewhere else for some reason
> > in
> > the future, we can make it configurable at build time (or something).
> >
> > -Corey
> >
> > Ok, here is the revised patch.
>
> Signed-off-by: Joseph Smith
>
> Thanks - Joe

Acked-by: Corey Osgood

Quoting Corey Osgood :
> On Sat, Apr 5, 2008 at 11:38 PM,  wrote:
> > PMBASE_ADDR can be anywhere we want it to be, so long as it doesn't
> > > collide
> > > with any IO addresses that have been reserved for other things.
> > > Currently,
> > > it's defined in i82801xx_lpc.c at the same address, that's why I was
> > > saying
> > > to move those defines as well (so PMBASE_ADDR isn't defined in 2
> > > different
> > > places). If PMBASE_ADDR ever needs to be somewhere else for some reason
> > > in
> > > the future, we can make it configurable at build time (or something).
> > >
> > > -Corey
> > >
> > > Ok, here is the revised patch.
> >
> > Signed-off-by: Joseph Smith
> >
> > Thanks - Joe
>
> Acked-by: Corey Osgood

r3218

Thanks - Joe