[coreboot] patch: all superios in v3 are set up wrong
Uwe Hermann
uwe at hermann-uwe.de
Fri Oct 3 18:54:08 CEST 2008
On Fri, Oct 03, 2008 at 09:15:37AM -0700, ron minnich wrote:
> Add the it8712f.
>
> Also, EVERY superio in v3 was set up wrong. The dts has to point to device
> operations (if there are any).
>
> Build tested on amd serengeti.
>
> Signed-off-by: Ronald G. Minnich <rminnich at gmail.com>
> Index: superio/ite/it8712f/stage1.c
> ===================================================================
> --- superio/ite/it8712f/stage1.c (revision 0)
> +++ superio/ite/it8712f/stage1.c (revision 0)
> @@ -0,0 +1,115 @@
> +/*
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2006 Uwe Hermann <uwe at hermann-uwe.de>
> + * Copyright (C) 2008 Ronald G. Minnich <rminnich at gmail.com>
--- Insert an empty line here, please (as in the other files) ----
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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
> + */
> +
> +#include <io.h>
> +#include <device/pnp.h>
> +#include "it8712f.h"
> +
> +/* The base address is 0x2e or 0x4e, depending on config bytes. */
> +#define SIO_BASE 0x2e
> +#define SIO_INDEX SIO_BASE
> +#define SIO_DATA SIO_BASE+1
FYI, the IT8712F code is (like many other superios in v2) quite a mess right
now, I'll send patches to fix various issues...
> Index: superio/ite/it8712f/superio.c
> ===================================================================
> --- superio/ite/it8712f/superio.c (revision 0)
> +++ superio/ite/it8712f/superio.c (revision 0)
> @@ -0,0 +1,144 @@
> +/*
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2006 Uwe Hermann <uwe at hermann-uwe.de>
> + * Copyright (C) 2007 Philipp Degler <pdegler at rumms.uni-mannheim.de>
> + * Copyright (C) 2008 Ronald G. Minnich <rminnich at gmail.com>
Don't add the (C) this time I'd say, the changes are too small to
warrant adding it (IMHO).
> +static void it8712f_pnp_set_resources(struct device * dev)
> +{
> + pnp_enter_ext_func_mode(dev);
> + pnp_set_resources(dev);
> + pnp_exit_ext_func_mode(dev);
> +}
> +
> +static void it8712f_pnp_enable_resources(struct device * dev)
> +{
> + pnp_enter_ext_func_mode(dev);
> + pnp_enable_resources(dev);
> + pnp_exit_ext_func_mode(dev);
> +}
> +
> +static void it8712f_pnp_enable(struct device * dev)
> +{
> + pnp_enter_ext_func_mode(dev);
> + pnp_set_logical_device(dev);
> + pnp_set_enable(dev, dev->enabled);
> + pnp_exit_ext_func_mode(dev);
> +}
> +
> +static void it8712f_setup_scan_bus(struct device *dev)
> +{
> + pnp_enable_devices(dev, &ops, ARRAY_SIZE(pnp_dev_info), pnp_dev_info);
I'm not sure about this -- what is 'ops'? Should it be it8712f_ops
here? I don't see any 'ops' being defined for this superio.
The IT8716F does this in v3:
static struct device_operations ops;
right before the
static struct pnp_info pnp_dev_info[] = {
[...]
> +}
> +
> +struct device_operations it8712f_ops = {
> + .phase2_setup_scan_bus = it8712f_setup_scan_bus,
> + .phase4_read_resources = pnp_read_resources,
> + .phase4_set_resources = it8712f_pnp_set_resources,
> + .phase4_enable_disable = it8712f_pnp_enable_resources,
> + .phase5_enable_resources = it8712f_pnp_enable,
> + .phase6_init = it8712f_init,
> +};
> +
> +/* TODO: FDC, MIDI, GAME, IR. */
> +static struct pnp_info pnp_dev_info[] = {
> + {&ops, IT8712F_SP1, PNP_IO0 | PNP_IRQ0, {0x7f8, 0}, },
> + {&ops, IT8712F_SP2, PNP_IO0 | PNP_IRQ0 | PNP_DRQ0 | PNP_DRQ1, {0x7f8, 0}, },
> + {&ops, IT8712F_PP, PNP_IO0 | PNP_IRQ0 | PNP_DRQ0, {0x07f8, 0},},
> + {&ops, IT8712F_EC, PNP_IO0 | PNP_IO1 | PNP_IRQ0, {0x7f8, 0}, {0x7f8, 0x4},},
> + {&ops, IT8712F_KBCK, PNP_IO0 | PNP_IO1 | PNP_IRQ0, {0x7f8, 0}, {0x7f8, 0x4}, },
> + {&ops, IT8712F_KBCM, PNP_IRQ0, },
> + {&ops, IT8712F_GPIO, },
> +};
> +
> +
> Index: superio/ite/it8712f/dts
> ===================================================================
> --- superio/ite/it8712f/dts (revision 0)
> +++ superio/ite/it8712f/dts (revision 0)
> @@ -0,0 +1,43 @@
> +/*
> + *
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2008 Peter Stuge <peter at stuge.se>
> + * Copyright (C) 2008 Ronald G. Minnich <rminnich at gmail.com>
(C) 2008 Ronald G. Minnich <rminnich at gmail.com> should be enough, this
is way too trivial to carry around other (C)'s...
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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
> + *
> + */
> +{
> + device_operations = "it8712f_ops";
> + /* COM1 */
> + com1dev = "1";
> + com1enable = "0";
> + com1io = "0x3f8";
> + com1irq = "4";
> +
> + /* COM2 */
> + com2dev = "2";
> + com2enable = "0";
> + com2io = "0x2f8";
> + com2irq = "3";
> +
> + /* Keyboard */
> + kbdev = "5";
> + kbenable = "0";
> + kbio = "0x60";
> + kbio2 = "0x62";
I think this should be 0x64.
> + kbirq = "1";
> + kbirq2 = "12";
And this should probably be "2", not "12".
Various devices are missing, I can send a patch later to fill them in.
Does is pose a problem if they're not all listed in the dts?
> +};
> Index: superio/ite/it8712f/it8712f.h
> ===================================================================
> --- superio/ite/it8712f/it8712f.h (revision 0)
> +++ superio/ite/it8712f/it8712f.h (revision 0)
> @@ -0,0 +1,35 @@
> +/*
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2006 Uwe Hermann <uwe at hermann-uwe.de>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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
> + */
> +
> +/* Datasheet: http://www.ite.com.tw/product_info/PC/Brief-IT8712_2.asp */
> +/* Status: Com1 is tested and works. */
> +
> +#define IT8712F_FDC 0x00 /* Floppy */
> +#define IT8712F_SP1 0x01 /* Com1 */
> +#define IT8712F_SP2 0x02 /* Com2 */
> +#define IT8712F_PP 0x03 /* Parallel port */
> +#define IT8712F_EC 0x04 /* Environment controller */
> +#define IT8712F_KBCK 0x05 /* Keyboard */
> +#define IT8712F_KBCM 0x06 /* Mouse */
> +#define IT8712F_GPIO 0x07 /* GPIO */
> +#define IT8712F_MIDI 0x08 /* MIDI port */
> +#define IT8712F_GAME 0x09 /* GAME port */
> +#define IT8712F_IR 0x0a /* Consumer IR */
> +
> Index: superio/ite/it8712f/Makefile
> ===================================================================
> --- superio/ite/it8712f/Makefile (revision 0)
> +++ superio/ite/it8712f/Makefile (revision 0)
> @@ -0,0 +1,29 @@
> +##
> +## This file is part of the coreboot project.
> +##
> +## Copyright (C) 2008 Peter Stuge <peter at stuge.se>
> +##
> +## This program is free software; you can redistribute it and/or modify
> +## it under the terms of the GNU General Public License as published by
> +## the Free Software Foundation; either version 2 of the License, or
> +## (at your option) any later version.
> +##
> +## 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
> +##
> +
> +ifeq ($(CONFIG_SUPERIO_ITE_IT8712F),y)
> +
> +STAGE0_CHIPSET_SRC += $(src)/superio/ite/it8712f/stage1.c
> +STAGE0_CHIPSET_SRC += $(src)/device/pnp_raw.c
> +
> +# Always add to variables, as there could be more than one Super I/O.
> +STAGE2_CHIPSET_SRC += $(src)/superio/ite/it8712f/superio.c
> +
> +endif
> Index: superio/ite/it8716f/superio.c
> ===================================================================
> --- superio/ite/it8716f/superio.c (revision 884)
> +++ superio/ite/it8716f/superio.c (working copy)
> @@ -164,7 +164,7 @@
> }
> }
>
> -static struct device_operations ops = {
> +struct device_operations it8716f_ops = {
> .phase2_setup_scan_bus = it8716f_setup_scan_bus,
> .phase4_read_resources = pnp_read_resources,
> .phase4_set_resources = it8716f_pnp_set_resources,
> Index: superio/ite/it8716f/dts
> ===================================================================
> --- superio/ite/it8716f/dts (revision 884)
> +++ superio/ite/it8716f/dts (working copy)
> @@ -20,6 +20,7 @@
> *
> */
> {
> + device_operations = "it8716f_ops";
> /* Floppy */
> floppydev = "0x0";
> floppyenable = "0";
Uwe.
--
http://www.hermann-uwe.de | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
More information about the coreboot
mailing list